Skip to content

Conversation

xDivisionByZerox
Copy link
Member

@xDivisionByZerox xDivisionByZerox commented Jan 22, 2023

Refactor faker.internet.userAgent, so that every return value is equally possible. That means that the function will no longer return values based on a "real-world" distribution.
I did this for multiple reasons:

  1. They are outdated anyway. Yes, you could update the probabilities, but that would require continuous maintenance (since those numbers change a lot)
  2. Faker is not a real-world database! We provide data which is reasonable. It is not our goal to mirrow the real world in all tiny details.
  3. We don't have a probability of distribution in other functions (and if so, that's most likely not that smart as well).
    For example: In Germany, the last name "Müller" is super popular, yet it has the same return probability as any other last name in the German locale dataset.

Could be seen as a fix for #216.

@xDivisionByZerox xDivisionByZerox added p: 1-normal Nothing urgent c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: internet Something is referring to the internet module labels Jan 22, 2023
@xDivisionByZerox xDivisionByZerox requested a review from a team January 22, 2023 12:48
@xDivisionByZerox xDivisionByZerox self-assigned this Jan 22, 2023
@xDivisionByZerox xDivisionByZerox requested a review from a team as a code owner January 22, 2023 12:48
@xDivisionByZerox xDivisionByZerox linked an issue Jan 22, 2023 that may be closed by this pull request
@xDivisionByZerox xDivisionByZerox changed the title Proposal: refactor(internet): remove weigths in userAgent refactor(internet): remove weigths in userAgent Jan 22, 2023
@codecov
Copy link

codecov bot commented Jan 22, 2023

Codecov Report

Merging #1761 (1def91a) into next (ac1a3de) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #1761   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files        2340     2340           
  Lines      242653   242619   -34     
  Branches     1111     1113    +2     
=======================================
- Hits       241804   241783   -21     
+ Misses        828      815   -13     
  Partials       21       21           
Impacted Files Coverage Δ
src/modules/internet/user-agent.ts 94.08% <100.00%> (+2.95%) ⬆️

@ST-DDT ST-DDT added the s: accepted Accepted feature / Confirmed bug label Jan 22, 2023
@xDivisionByZerox xDivisionByZerox requested a review from a team January 23, 2023 15:21
@matthewmayer
Copy link
Contributor

The whole random_ua functions seem like overkill, and is hard to maintain (e.g. there's no Edge). Could it just be replaced by a random pick from a static array of popular UAs like:

https://gist.github.com/fijimunkii/952acac988f2d25bef7e0284bc63c406?permalink_comment_id=4447237#gistcomment-4447237

@xDivisionByZerox
Copy link
Member Author

The whole random_ua functions seem like overkill, and is hard to maintain [...]

Most definitely. I had some other refactoring changes in mind to make this more maintainable in the future.
But we might as well replace it with a locale entry.

@Shinigami92 Shinigami92 requested a review from ejcheng January 28, 2023 15:22
@ST-DDT ST-DDT enabled auto-merge (squash) January 29, 2023 15:40
@ST-DDT ST-DDT merged commit cec7877 into next Jan 29, 2023
@ST-DDT ST-DDT deleted the refactor/user-agent/remove-weights branch January 29, 2023 15:44
matthewmayer pushed a commit to matthewmayer/faker that referenced this pull request Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: internet Something is referring to the internet module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

RFC: Refactor random User Agents
4 participants