feat(ngx-deploy-npm): add version check to prevent duplicate publishes #667
Conversation
packages/ngx-deploy-npm/src/executors/deploy/engine/engine.spec.ts
Outdated
Show resolved
Hide resolved
|
Hi @dianjuar, thanks for the suggestions. I replaced the readFileSync and removed the beforeEach |
There was a problem hiding this comment.
@Romain7495 Thank you so much for taking the initiative to implement this feature.
It has an excellent foundation for the feature. Thank you so much for that.
Some concerns exist about the implementation and the flexibility to adapt to other workflow necessities. I started a discussion on issue #666.
|
@dianjuar agree, I have updated the type of checkExisting and unit test, It works as welll on our nx monorepo with |
Romain7495
force-pushed
the
main
branch
2 times, most recently
from
d52e512 to
56f81eb
Compare
|
hello @dianjuar, do you think we can merge this ? do you need something else ? Do you need help to fix the sonarcloud pipeline issue (SONAR_TOKEN missing ? ) |
There was a problem hiding this comment.
I have more suggestions about the unit test.
Thanks for your patience in waiting for my feedback.
I use this time Conventional Comments to express my feedback in a more effective way.
packages/ngx-deploy-npm/src/executors/deploy/engine/engine.spec.ts
Outdated
Show resolved
Hide resolved
packages/ngx-deploy-npm/src/executors/deploy/engine/engine.spec.ts
Outdated
Show resolved
Hide resolved
packages/ngx-deploy-npm/src/executors/deploy/engine/engine.spec.ts
Outdated
Show resolved
Hide resolved
packages/ngx-deploy-npm/src/executors/deploy/engine/engine.spec.ts
Outdated
Show resolved
Hide resolved
packages/ngx-deploy-npm/src/executors/deploy/engine/engine.spec.ts
Outdated
Show resolved
Hide resolved
|
@Romain7495 regarding that "failing" check on the CI. Don't mind it. I'll try to fix it in the next couple of hours. It has been a problem since its implementation. It's failing for contributors, but it works fine for repo members. Ideally, it would trigger a static code analysis of your contribution to detect unit test coverage, code smells, and security vulnerabilities. If I can't find a fix, I would integrate it with it failing. |
Romain7495
force-pushed
the
main
branch
4 times, most recently
from
cc6cfe3 to
e538aa1
Compare
|
@dianjuar Thanks for all the feedback! First time doing Jest tests, so I really appreciate the tips. Pretty sure I got all your suggestions covered in my latest changes 😊 |
|
@Romain7495
|
This commit adds a new optional flag `checkExisting` that allows users to verify if a package version already exists in the registry before publishing. When enabled, it will: - Check the package.json for name and version - Query npm registry for existing versions - Skip publishing if version already exists - Default to false to maintain backward compatibility and avoid regression - Add test - Add documentation This helps prevent accidental republishing package and having issue (403) when trying to republish a version that is already pushed (on gitlab for example) Signed-off-by: Romain LABAT <[email protected]>
fixes: #666
fixes: #508
This commit adds a new optional flag
checkExistingthat allows users to verify if a package version already exists in the registry before publishing. When enabled, it will:This helps prevent accidental republishing package and having issue (403) when trying to republish a version that is already pushed (on gitlab for example)
PR Checklist
Please check if your PR fulfills the following requirements:
What is the current behavior?
Issue Number: #666
What is the new behavior?
Does this PR introduce a breaking change?
Other information