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

fix: Remove duplicates in config warnings #6443

Merged
merged 6 commits into from
May 15, 2023
Merged

Conversation

kashyapkaki
Copy link
Contributor

While updating lockfileVersion other than the values null,1,2,3 it's showing a warning with duplicate lock file versions as below.

image

Issue exists in definitions of config utils. I removed the duplicates to fix this issue. Please review and let me know your feedback.

References

Fixes #6404

@kashyapkaki kashyapkaki requested a review from a team as a code owner May 11, 2023 12:02
@wraithgar
Copy link
Member

npm run lintfix should get things cleaned up automatically for you.

@wraithgar
Copy link
Member

This is a great start. Typically single use functions are discouraged because of the extra mental load it takes to follow but I think it's probably appropriate here due to all the branching paths it takes.

Just two small suggestions, linting, and it looks good.

Can you run npm run snap in the main cli too? At least some test should change here I would hope.

@kashyapkaki
Copy link
Contributor Author

Hi @wraithgar, have addressed those small suggestions and fixed linting issues as well.

I did ran npm run snap command, here are the results below.

image

I can see 4 tests failed in publish.js. However, these tests are failing even without my changes. So, please suggest me what to do on this.

@wraithgar
Copy link
Member

That's fine, we'll dig into any missing test coverage on this if we need to later. Tested locally and it looks like it should, and other config tests hit those lines already so we have coverage.

@wraithgar
Copy link
Member

Sorry I missed that indexOf/includes suggestion before. That should be the last thing.

@kashyapkaki
Copy link
Contributor Author

Hi @wraithgar, have addressed the above changes. Please review and let me know. Thank you!

@wraithgar wraithgar changed the title fix: Removed duplicate lock file versions fix: Remove duplicates in config warnings May 15, 2023
@wraithgar wraithgar merged commit 7ade93d into npm:latest May 15, 2023
@wraithgar wraithgar self-assigned this May 15, 2023
@github-actions github-actions bot mentioned this pull request May 15, 2023
@github-actions github-actions bot mentioned this pull request Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] npm invalid config warning duplicates lock file version
3 participants