Skip to content

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

Merged
Shinigami92 merged 9 commits intomainfrom
hacker_abbreviation
Jun 18, 2022
Merged

feat: add abbreviations to hacker.abbreviation() in the en locale#1086
Shinigami92 merged 9 commits intomainfrom
hacker_abbreviation

Conversation

@ejcheng
Copy link
Copy Markdown
Member

@ejcheng ejcheng commented Jun 17, 2022

some logic gates, other acronyms and abbreviations

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

codecov Bot commented Jun 17, 2022 •
edited
Loading

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
Copy Markdown
Member

xDivisionByZerox commented Jun 17, 2022

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

xDivisionByZerox
xDivisionByZerox reviewed Jun 17, 2022
View reviewed changes
Comment thread src/locales/en/hacker/abbreviation.ts
@ejcheng ejcheng 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
@ejcheng @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>
@ejcheng ejcheng force-pushed the hacker_abbreviation branch from 22ed0d6 to 582283e Compare June 18, 2022 05:42
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
Copy Markdown
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
Copy Markdown
Member

xDivisionByZerox commented Jun 18, 2022

SSD!

What? We already have that.

@ST-DDT
Copy link
Copy Markdown
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
Copy Markdown
Member

xDivisionByZerox commented Jun 18, 2022 •
edited
Loading

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
Copy Markdown
Member

ST-DDT commented Jun 18, 2022

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

@ST-DDT
Copy link
Copy Markdown
Member

ST-DDT commented Jun 18, 2022

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

@ejcheng @ST-DDT
chore: sort abbreviations alphabetically
a945acc 
Co-authored-by: ST-DDT <st-ddt@gmx.de>
@ejcheng ejcheng requested a review from ST-DDT June 18, 2022 15:23
Shinigami92
Shinigami92 reviewed Jun 18, 2022
View reviewed changes
Comment thread src/locales/en/hacker/abbreviation.ts
ST-DDT
ST-DDT previously approved these changes Jun 18, 2022
View reviewed changes
@ejcheng @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
@ejcheng ejcheng removed the needs rebase There is a merge conflict label Jun 18, 2022
@ejcheng
Copy link
Copy Markdown
Member Author

ejcheng commented Jun 18, 2022 •
edited
Loading

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
Copy Markdown
Member

Shinigami92 commented Jun 18, 2022

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
@ejcheng
Copy link
Copy Markdown
Member Author

ejcheng commented Jun 18, 2022

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
@ejcheng @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

@ejcheng ejcheng

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.

4 participants

@ejcheng @xDivisionByZerox @ST-DDT @Shinigami92