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.

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

feat: add internet.emoji #504

Merged
merged 26 commits into from
Apr 19, 2022
Merged

Conversation

hugoattal
Copy link
Contributor

@hugoattal hugoattal commented Feb 17, 2022

fix: #497

So I added a function faker.internet.emoji() that generate emojis.
You can also filter only some groups of emoji with faker.internet.emoji(["food"]) for example.

I took all the fully-qualified emoji of Unicode : https://unicode.org/Public/emoji/14.0/emoji-test.txt

I didn't separate each subgroup, as I don't think people need that much granularity.
I also disabled prettier on the emoji.ts file to have a more readable file so that you can spot the group names, but I can keep the prettier format if you prefer (I will have a huge lines number contribution 😛 !).

For the test, I only check if the string has a length of at least 1, as the regex to check emoji are quite hacky for now.
I found using a package (such as this one: https://github.com/mathiasbynens/emoji-regex) only for this test to be unnecessary.

@hugoattal hugoattal requested a review from a team as a code owner February 17, 2022 21:54
@ST-DDT ST-DDT added the c: feature Request for new feature label Feb 17, 2022
@ST-DDT ST-DDT added this to the v6.2 - New small features milestone Feb 17, 2022
@ST-DDT ST-DDT requested a review from a team February 17, 2022 22:40
@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #504 (8173b49) into main (2264314) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 8173b49 differs from pull request most recent head 976946e. Consider uploading reports for the commit 976946e to get more accurate results

@@            Coverage Diff             @@
##             main     #504      +/-   ##
==========================================
+ Coverage   99.34%   99.35%   +0.01%     
==========================================
  Files        1921     1922       +1     
  Lines      179522   182746    +3224     
  Branches      898      899       +1     
==========================================
+ Hits       178345   181572    +3227     
+ Misses       1121     1118       -3     
  Partials       56       56              
Impacted Files Coverage Δ
src/locales/en/internet/emoji.ts 100.00% <ø> (ø)
src/definitions/internet.ts 100.00% <100.00%> (ø)
src/internet.ts 100.00% <100.00%> (ø)
src/locales/en/internet/index.ts 100.00% <100.00%> (ø)
src/locales/fa/internet/index.ts 100.00% <100.00%> (ø)
src/faker.ts 100.00% <0.00%> (ø)
src/definitions/definitions.ts 100.00% <0.00%> (ø)
src/locales/ar/name/male_first_name.ts 100.00% <0.00%> (ø)
src/locales/pl/address/street_prefix.ts 100.00% <0.00%> (ø)
src/utils/user-agent.ts 98.92% <0.00%> (+0.80%) ⬆️

@Shinigami92 Shinigami92 added p: 1-normal Nothing urgent and removed c: feature Request for new feature labels Feb 18, 2022
src/definitions/internet.ts Outdated Show resolved Hide resolved
src/internet.ts Outdated Show resolved Hide resolved
@import-brain import-brain added the c: feature Request for new feature label Feb 18, 2022
@import-brain import-brain added the needs rebase There is a merge conflict label Mar 27, 2022
@Shinigami92
Copy link
Member

We're starting work on the v6.2 milestone, but this PR needs a rebase.
Please rebase and resolve all conflicts. Until then, we're moving it to the v6 milestone, which now serves as the "v6-Backlog".
If this PR no longer contains any conflicts and can certainly be a target for the next release, we will move it to the then active milestone v6.x.

@hugoattal
Copy link
Contributor Author

hugoattal commented Apr 5, 2022

Yep sorry, kinda forgot about it... I'll do the rebase and the updates tomorrow!

EDIT: ok I lied. I'm doing it RIGHT NOW!

@hugoattal hugoattal requested a review from a team April 8, 2022 23:46
@hugoattal hugoattal force-pushed the add-internet-emoji branch from 00a4cf1 to e2dcf98 Compare April 8, 2022 23:52
@hugoattal
Copy link
Contributor Author

Sorry for the little mess in the commits, first time I rebased a forked repo 😅...
Everything should be good now 🙂

@hugoattal hugoattal requested review from ST-DDT and Shinigami92 April 9, 2022 00:16
@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed needs rebase There is a merge conflict labels Apr 9, 2022
ST-DDT
ST-DDT previously approved these changes Apr 9, 2022
@ST-DDT ST-DDT requested review from a team April 9, 2022 01:00
@hugoattal hugoattal requested a review from Shinigami92 April 10, 2022 17:19
src/internet.ts Outdated Show resolved Hide resolved
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.

I found some non-emoji emojis 🤔 Not sure what to do with them or it is my system and a non-installed font
Did not saw them before, cause it was just a large chunk of icons. Now were everything is in it's own line, it's much better readable 🙂

src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
src/locales/en/internet/emoji.ts Show resolved Hide resolved
src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
src/locales/en/internet/emoji.ts Outdated Show resolved Hide resolved
@Shinigami92
Copy link
Member

45 and 46 are duplicates but now removed both 🤔
51 and 68 was okay but removed now?
80 was correctly removed

I'm not sure if the unqualified is the correct "filter"

image

@hugoattal
Copy link
Contributor Author

@Shinigami92 You're right some were missing, I fixed it here: 4e239d7

src/internet.ts Outdated Show resolved Hide resolved
@ST-DDT
Copy link
Member

ST-DDT commented Apr 18, 2022

I'm very busy today, so I will have a look at this tomorrow.

@Shinigami92 Shinigami92 enabled auto-merge (squash) April 19, 2022 20:23
@Shinigami92 Shinigami92 dismissed pkuczynski’s stale review April 19, 2022 20:23

outdated change request

@Shinigami92 Shinigami92 merged commit cb746cb into faker-js:main Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a faker.internet.emoji function
6 participants