-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Conversation
Also refactored some of the ternary operators and removed unnecessary code.
|
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 |
Hi @wraithgar, have addressed those small suggestions and fixed linting issues as well. I did ran 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. |
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. |
Sorry I missed that indexOf/includes suggestion before. That should be the last thing. |
Hi @wraithgar, have addressed the above changes. Please review and let me know. Thank you! |
While updating
lockfileVersion
other than the valuesnull,1,2,3
it's showing a warning with duplicate lock file versions as below.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