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
feat(align-deps) allow exact version within range according to semver… #2984
base: main
Are you sure you want to change the base?
Conversation
I marked the PR as Draft as I haven't finished with the implementation, e.g. I still need to add unit tests but I wanted to ask the project maintainers' opinion (especially @tido64) considering this is my first feature addition to this repo. I appreciate any help and suggestion! |
thanks for sending it over! |
packages/align-deps/src/cli.ts
Outdated
"allow-exact-version": { | ||
default: false, | ||
description: | ||
"Determines whether align-deps should allow exact versions in 'package.json' that are within the range and not throw an error for them.", | ||
type: "boolean", | ||
}, | ||
}; |
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.
nit: once everything else is handled and we're good on the implementation, we should add mention of this in the readme: https://github.com/microsoft/rnx-kit/blob/main/packages/align-deps/README.md#usage
Sure, that's definitely on my list. You know what, I'll update the PR description with checkboxes for the remaining tasks and if I forgot something you can call that out! Getting back to changeset, what kind of version bump you think this change warrants? |
@tido64 any preferences? If we want to be close to the semver definition I'd say we should go with minor:
since it's a new feature |
@balazsgerlei: Are you unblocked to continue working on this? If you don't have the time, let me know. I wouldn't mind taking over and pushing it over the finish line. 😄 |
I've been indeed very occupied with personal stuff so I had little to no free time after work to attend to this. It would be still a nice challenge to finish this so I'll try to allocate some time for it and if I couldn't do it this week I'll let you know, but thanks for offering to take over! |
Take the time you need to finish this. No pressure. Just making sure you're not waiting to hear from us 😉 |
94eb7a2
to
b424f3e
Compare
Okay, it took a while, I've finally had some time to update this. I implemented the two distinct diff mode options, what I'm not sure about is how and where to handle if the option is not set correctly. It should not happen since the only two valid options are delcared in cli.ts though I feel like it should still be handled in e.g. check.ts. I could add a simple Can I ask for some guidence on this @tido64 @kelset ? Thank you! |
Hey @balazsgerlei Tommy is currently on a break; I'll throw in my 2C$ in the meantime. Because we are setting the options via What happens currently if you try to run the command and passing an invalid option? |
The available options are listed and at the end it also tells you what you did wrong:
That's what I also meant is that when using the script in Terminal you basically cannot pass the wrong argument to the actual logic, just that it would be more precise to handle that case somewhere e.g. in So I think I will continue with the unit tests like this and in the time being, let me know if you have another solution in mind or if @tido64 comes back and has a different opinion! |
60f3451
to
650df3d
Compare
650df3d
to
fcbdc5a
Compare
Thank you for the PR, we also need that feature in our team. I was about to implement that myself until i saw this PR 😄. I have one small suggestion that would make this ideal for us, currently the The following would be valid then: |
…Satisfies
Description
Add a new option (flag)
--diff-mode
to allow specifying two modes for diffing:strict
: Same as beforeallow-exact-versions
: Allow (don't throw an error) when exact versions are declared inpackage.json
that are within range according tosemverSatisfies
.Resolves #2983
Remaining tasks:
diff-mode
options from a boolean flagTest plan
package.json
with an exact version that is within the range of what the built-in capabilities define. Runalign-deps
with the new--diff-mode
set toallow-exact-version
and verify that no error is thrown.package.json
with an exact version that is within the range of what the built-in capabilities define. Runalign-deps
with the new--diff-mode
set toallow-exact-version
and--write
flag set and verify that nothing changes in package.json.package.json
with an exact version that is outside of the range of what the built-in capabilities define. Runalign-deps
with the new--diff-mode
set toallow-exact-version
and verify that an error is thrown for not correct version.package.json
with an exact version that is within the range of what the built-in capabilities define. Runalign-deps
with the new--diff-mode
set tostrict
and verify that an error is thrown for not correct version.package.json
with an exact version that is within the range of what the built-in capabilities define. Runalign-deps
with the new--diff-mode
set tostrict
but with--write
flag and verify that the versions is overwritten with what is defined in built-in capabilities.package.json
with an exact version that is outside of the range of what the built-in capabilities define. Runalign-deps
with the new--diff-mode
set tostrict
and verify that an error is thrown for not correct version.package.json
with an exact version that is within the range of what the built-in capabilities define. Runalign-deps
without the new--diff-mode
set and verify that an error is thrown for not correct version.package.json
with an exact version that is within the range of what the built-in capabilities define. Runalign-deps
without the new--diff-mode
set but with--write
flag and verify that the versions is overwritten with what is defined in built-in capabilities.package.json
with an exact version that is outside of the range of what the built-in capabilities define. Runalign-deps
without the new--diff-mode
set and verify that an error is thrown for not correct version.