Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add abbreviations to hacker.abbreviation() in the en locale #1086

Merged
merged 9 commits into from Jun 18, 2022
Merged

feat: add abbreviations to hacker.abbreviation() in the en locale #1086

merged 9 commits into from Jun 18, 2022

Conversation

import-brain
Copy link
Member

some logic gates, other acronyms and abbreviations

@import-brain
feat: add abbreviations to hacker.abbreviation()
b47a541 
logic gates, some other acronyms and abbreviations
@import-brain import-brain added p: 1-normal Nothing urgent c: locale Permutes locale definitions labels Jun 17, 2022
@import-brain import-brain added this to the v7 - Current Major milestone Jun 17, 2022
@import-brain import-brain requested a review from a team June 17, 2022 19:22
@import-brain import-brain self-assigned this Jun 17, 2022
@import-brain import-brain requested a review from a team as a code owner June 17, 2022 19:22
@codecov
Copy link

codecov bot commented Jun 17, 2022 •
edited

Codecov Report

Merging #1086 (7d1fa12) into main (1898f28) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1086      +/-   ##
==========================================
- Coverage   99.65%   99.64%   -0.01%     
==========================================
  Files        2146     2146              
  Lines      230380   230393      +13     
  Branches      982      978       -4     
==========================================
- Hits       229588   229578      -10     
- Misses        771      794      +23     
  Partials       21       21
Impacted Files Coverage Δ
src/locales/en/hacker/abbreviation.ts 100.00% <100.00%> (ø)
src/modules/internet/user-agent.ts 86.95% <0.00%> (-5.80%) ⬇️
src/modules/finance/index.ts 99.31% <0.00%> (-0.69%) ⬇️

@xDivisionByZerox
Copy link
Member

Aren't the "logic gates" just plain words? I'm questioning the decision to put them in the abbreviations file.

@import-brain import-brain changed the title locale: add abbreviations to hacker.abbreviation() in the en locale feat: add abbreviations to hacker.abbreviation() in the en locale Jun 18, 2022
@import-brain @xDivisionByZerox @ST-DDT
chore: apply suggestions from code review
582283e 
Co-authored-by: xDivisionByZerox <leyla.jaehnig@gmx.de>
Co-authored-by: ST-DDT <st-ddt@gmx.de>
ST-DDT
ST-DDT previously approved these changes Jun 18, 2022
View reviewed changes
Shinigami92
Shinigami92 reviewed Jun 18, 2022
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSD!

@xDivisionByZerox
Copy link
Member

SSD!

What? We already have that.

@ST-DDT
Copy link
Member

ST-DDT commented Jun 18, 2022

SSD!

What? We already have that.

Can you give me a line number?
I only see SDD (Same Day Delivery?).

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jun 18, 2022 •
edited

Can you give me a line number? I only see SDD (Same Day Delivery?).

Damn, your write. I literally read that as SSD...

Well in that case we can add 'HDD' as well.

Edit: Well that on the other side already is existing...

@ST-DDT
Copy link
Member

ST-DDT commented Jun 18, 2022

Looks like your brain is melting just like mine 😉😅🥵

@ST-DDT
Copy link
Member

ST-DDT commented Jun 18, 2022

Please either sort them alphabetically or sort them by their category as before.

@import-brain @ST-DDT
chore: sort abbreviations alphabetically
a945acc 
Co-authored-by: ST-DDT <st-ddt@gmx.de>
@import-brain import-brain requested a review from ST-DDT June 18, 2022 15:23
ST-DDT
ST-DDT previously approved these changes Jun 18, 2022
View reviewed changes
@import-brain @Shinigami92
Update src/locales/en/hacker/abbreviation.ts
2dc17f9 
Co-authored-by: Shinigami <chrissi92@hotmail.de>
Shinigami92
Shinigami92 previously approved these changes Jun 18, 2022
View reviewed changes
@Shinigami92 Shinigami92 dismissed their stale review June 18, 2022 15:32

tests are failing

Shinigami92
Shinigami92 previously approved these changes Jun 18, 2022
View reviewed changes
@Shinigami92 Shinigami92 added the needs rebase There is a merge conflict label Jun 18, 2022
@import-brain import-brain removed the needs rebase There is a merge conflict label Jun 18, 2022
@import-brain
Copy link
Member Author

import-brain commented Jun 18, 2022 •
edited

I don't know if I'm doing this correctly or not, but the results that the seeded tests return seem to change on every run of the tests now that there are snapshots. I've manually changed them multiple times, but the received value from the tested function seems to change every time tests are run. Also, updating the snapshot by pressing "u" after the tests run doesn't seem to do anything, what's up with that?

@Shinigami92
Copy link
Member

You left the seededRuns object and didn't remove it

@Shinigami92 Shinigami92 enabled auto-merge (squash) June 18, 2022 16:27
@Shinigami92 Shinigami92 merged commit c5d8934 into main Jun 18, 2022
@import-brain
Copy link
Member Author

You left the seededRuns object and didn't remove it

Ahhh, I see. Thanks Shini :)

@ST-DDT ST-DDT deleted the hacker_abbreviation branch June 18, 2022 17:02
Minozzzi pushed a commit to Minozzzi/faker that referenced this pull request Jul 19, 2022
@import-brain @Shinigami92 @Minozzzi
feat: add abbreviations to hacker.abbreviation() in the en locale (…
56c271e 
…faker-js#1086)

Co-authored-by: Shinigami <chrissi92@hotmail.de>
@xDivisionByZerox xDivisionByZerox added the m: hacker Something is referring to the hacker module label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ST-DDT ST-DDT ST-DDT approved these changes

@Shinigami92 Shinigami92 Shinigami92 approved these changes

@xDivisionByZerox xDivisionByZerox Awaiting requested review from xDivisionByZerox

Assignees

@import-brain import-brain

Labels
c: locale Permutes locale definitions m: hacker Something is referring to the hacker module p: 1-normal Nothing urgent
Projects
No open projects
Faker Roadmap
Status: Done
Milestone
v7 - Current Major
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@import-brain @xDivisionByZerox @ST-DDT @Shinigami92