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(internet): add jwt method #2936

Merged
merged 40 commits into from
Oct 23, 2024
Merged

feat(internet): add jwt method #2936

merged 40 commits into from
Oct 23, 2024

Conversation

eLoyyyyy
Copy link
Contributor

@eLoyyyyy eLoyyyyy commented Jun 4, 2024

Closes #1282

@eLoyyyyy eLoyyyyy requested a review from a team as a code owner June 4, 2024 10:24
Copy link

netlify bot commented Jun 4, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 5c3f33d
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/6718f231d852a600084e2d8b
😎 Deploy Preview https://deploy-preview-2936.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent m: internet Something is referring to the internet module labels Jun 4, 2024
@ST-DDT ST-DDT added this to the vAnytime milestone Jun 4, 2024
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Thanks for contributing.

Could you please also comment in the linked issue so we can assign it to you so it easier to see that the issue is being worked on?

src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 marked this pull request as draft June 4, 2024 15:34
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
test/modules/internet.spec.ts Outdated Show resolved Hide resolved
test/modules/internet.spec.ts Outdated Show resolved Hide resolved
test/modules/internet.spec.ts Outdated Show resolved Hide resolved
test/modules/internet.spec.ts Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 91.37931% with 5 lines in your changes missing coverage. Please review.

Project coverage is 99.95%. Comparing base (48931a5) to head (5c3f33d).
Report is 1 commits behind head on next.

Files with missing lines Patch % Lines
src/internal/base64.ts 37.50% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2936      +/-   ##
==========================================
- Coverage   99.96%   99.95%   -0.01%     
==========================================
  Files        2796     2797       +1     
  Lines      216704   216762      +58     
  Branches      953      578     -375     
==========================================
+ Hits       216628   216672      +44     
- Misses         76       90      +14     
Files with missing lines Coverage Δ
src/definitions/internet.ts 100.00% <ø> (ø)
src/locales/base/internet/index.ts 100.00% <100.00%> (ø)
src/locales/base/internet/jwt_algorithm.ts 100.00% <100.00%> (ø)
src/modules/internet/index.ts 100.00% <100.00%> (ø)
src/internal/base64.ts 33.33% <37.50%> (+3.33%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Other than the following minor issues and the Buffer issue above, this looks good to me.

src/definitions/internet.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
@Shinigami92
Copy link
Member

is this up for review? or should I wait a bit?

@Shinigami92 Shinigami92 marked this pull request as ready for review June 13, 2024 15:05
Copy link
Member

@xDivisionByZerox xDivisionByZerox left a comment

Choose a reason for hiding this comment

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

Good work so far 👍

src/locales/base/internet/jwt_algorithm.ts Outdated Show resolved Hide resolved
src/definitions/internet.ts Show resolved Hide resolved
src/modules/internet/util.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Show resolved Hide resolved
src/modules/internet/index.ts Show resolved Hide resolved
@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jun 18, 2024

Discussion point has been removed for being off topic


@eLoyyyyy the test snapshots as well as some formatting needs to be updated. You can run pnpm run preflight to run CI locally before committing. That way you don't have to wait for results from CI. Alternativly, have a look at our CONTRIBUTING Guidelines for more information on how to fix common problems.

@ST-DDT

This comment was marked as off-topic.

@eLoyyyyy
Copy link
Contributor Author

@eLoyyyyy the test snapshots as well as some formatting needs to be updated. You can run pnpm run preflight to run CI locally before committing. That way you don't have to wait for results from CI. Alternativly, have a look at our CONTRIBUTING Guidelines for more information on how to fix common problems.

Updated the test snapshots. My bad for not running preflight before pushing code.

Copy link
Member

@xDivisionByZerox xDivisionByZerox left a comment

Choose a reason for hiding this comment

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

Looks really good. I have questions/suggestions for one last time.

src/modules/internet/util.ts Outdated Show resolved Hide resolved
src/modules/internet/util.ts Outdated Show resolved Hide resolved
@xDivisionByZerox

This comment was marked as off-topic.

ST-DDT
ST-DDT previously approved these changes Jun 22, 2024
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. ❤️

@ST-DDT ST-DDT requested review from xDivisionByZerox and a team June 22, 2024 13:38
@eLoyyyyy eLoyyyyy requested a review from ST-DDT October 16, 2024 12:40
@ST-DDT ST-DDT removed the needs rebase There is a merge conflict label Oct 16, 2024
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/internal/base64.ts Outdated Show resolved Hide resolved
@eLoyyyyy eLoyyyyy requested a review from ST-DDT October 17, 2024 05:53
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

A few minor tweaks and then this is good to go.

src/internal/base64.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
src/modules/internet/index.ts Outdated Show resolved Hide resolved
@eLoyyyyy eLoyyyyy requested a review from ST-DDT October 17, 2024 11:28
@ST-DDT ST-DDT requested review from xDivisionByZerox and a team October 17, 2024 11:46
@ST-DDT
Copy link
Member

ST-DDT commented Oct 23, 2024

@xDivisionByZerox Could you please review this PR (or dismiss your previous one)?

@ST-DDT ST-DDT merged commit e3858f2 into faker-js:next Oct 23, 2024
23 checks passed
@ST-DDT
Copy link
Member

ST-DDT commented Oct 23, 2024

Thank you for you contribution. ❤️

This feature will be available in the upcoming v9.1 release of faker.

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 m: internet Something is referring to the internet module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add jwt to internet module
4 participants