-
Notifications
You must be signed in to change notification settings - Fork 6
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
chore(atomic): split create-atomic
and its template altogether
#714
Conversation
Thanks for your contribution @louis-bompart !
|
Pull Request Report PR Title ✅ Title follows the conventional commit spec. |
cbd76e0
to
528ea10
Compare
ed0f418
to
3d1e324
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny how snyk got us there, template shouldn't even be a sub-package 😄
On hold, waiting for npm/map-workspaces#32 to be merged & released. Then for |
Yeah, it is true that from a technical standpoint this is unnecessary, one could even say silly, I totally agree. However, the boons it brings outweigh the cost by far imo; thanks to this pattern we are able to ensure the template we ship to our customer is using 'just the right' versions: not the past one, nor the future one. Dependencies bundled/related to the template are also kept to date, and we also ensure that there are no malicious dependencies shenanigans going on. The sub-packaging also allows the template to be 'just right' on publication thanks to topological releases. |
This reverts commit 1561a08.
Motivation
In npm/cli#4518, I found out that 'sub-packages' with npm workspaces are not supported.
Decision
From there two solutions:
Pro for 1:
Con for 1:
Pro for 2:
Con for 2:
Conclusion
I chose to go with option 2 because it was really simpler to put in place, and I think long-term it will help to distinguish between the 'leg-work' done by the generator, and what's actually generated.
It's important to note that the 'private' fields of the packages did not change, meaning that the template is still unpublished and that it's still bundled with
@coveo/create-atomic
, so no changes for the end-user.Testing
No logic changes, the bundling process will be tested by the CI.