Skip to content

chore(parser): remove faker dependency #3476

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

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

svozza
Copy link
Contributor

@svozza svozza commented Jan 16, 2025

Summary

Remove thes fake-jsr dependency from the parser package.

Changes

  • Remove faker from parser package.json
  • Update two test cases using the faker-js lib.
  • Regenerate pacakge-lcok.json file

Reviewer in #3388 requested we remove library completely.

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: #3480


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@svozza svozza requested a review from a team January 16, 2025 12:33
@svozza svozza requested a review from a team as a code owner January 16, 2025 12:33
@boring-cyborg boring-cyborg bot added dependencies Changes that touch dependencies, e.g. Dependabot, etc. tests PRs that add or change tests labels Jan 16, 2025
@pull-request-size pull-request-size bot added the size/XS PR between 0-9 LOC label Jan 16, 2025
Copy link

sonarqubecloud bot commented Jan 16, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copy link
Contributor

No related issues found. Please ensure there is an open issue related to this change to avoid significant delays or closure.

@github-actions github-actions bot added do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels Jan 16, 2025
@am29d am29d added parser This item relates to the Parser Utility and removed do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels Jan 16, 2025
@dreamorosi dreamorosi added the do-not-merge This item should not be merged label Jan 16, 2025
@am29d am29d requested a review from dreamorosi January 16, 2025 14:15
@dreamorosi
Copy link
Contributor

Hi @svozza, thanks for the PR and for the bias for action.

To make the project as transparent and approachable as possible, and to make it possible for us maintainers to keep track of things that get merged and why, we enforce that every PR must have a previously green-lit issue linked to it.

I recognize that in cases such as this one it might seem overkill, but for what it's worth, if we had a discussion in the issue I would have clarified that we wanted to remove both this dependency and the other (@anatine/zod-mock) which is a bit of a more involved change.

In the previous PR I decided to not say anything even though I should have, mainly because we had made you wait weeks to review the PR, but in the spirit of collaboration I think it's important that all contributors (us maintainers included) respect the conventions of the repository.

For this last time I am going to open an issue, link it, and then merge the PR.

@dreamorosi dreamorosi removed the do-not-merge This item should not be merged label Jan 16, 2025
@dreamorosi dreamorosi merged commit e1bc6c5 into aws-powertools:main Jan 16, 2025
54 of 55 checks passed
@dreamorosi dreamorosi linked an issue Jan 16, 2025 that may be closed by this pull request
2 tasks
@svozza
Copy link
Contributor Author

svozza commented Jan 16, 2025

No worries, I will do it next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Changes that touch dependencies, e.g. Dependabot, etc. parser This item relates to the Parser Utility size/XS PR between 0-9 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maintenance: remove @faker-js/faker dependency
3 participants