Sort packages alphabetically in changeset add prompt - #1122
Conversation
Sorry, something went wrong.
🦋 Changeset detectedLatest commit: 14ce52f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Sorry, something went wrong.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ae3fd61:
|
Sorry, something went wrong.
| { | ||
| name: "changed packages", | ||
| choices: changedPackages, | ||
| choices: changedPackages.sort(), |
There was a problem hiding this comment.
this should sort using the .name property of each package
Sorry, something went wrong.
There was a problem hiding this comment.
The changedPackages is actually already an array of strings (names).
Sorry, something went wrong.
| ); | ||
| }); | ||
|
|
||
| it("should sort packages alphabetically in the prompt", async () => { |
There was a problem hiding this comment.
That likely won't test this behavior. I would probably create such layout to test it:
packages-1/pkg-b/package.json packages-2/pkg-a/package.json
The point here is that pkg-a is contained in a directory that comes after the directory in which pkg-b is contained.
Sorry, something went wrong.
There was a problem hiding this comment.
You are right, thanks to a different mistake (the packages location did not match packages names), it seemed like it was doing the right thing, but not really.
I created 2 testing scenarios, one for changedPackages and one for unchangedPackages and named the directories packages-1/packages-2 as suggested. It seems like the unchangedPackages are already sorting alphabetically, so the new .sort call does not really change anything, but I feel like it is still a good reassurance, that it will still work even if something in the background changes.
I tried removing the .sort call and at least for changedPackages use-case the tests failed, so it seems to be working.
Sorry, something went wrong.
|
@Andarist Thanks for the quick review, I updated the test, so hopefully it will work as expected. Feel free to check it when you have some more time :) |
Sorry, something went wrong.
bluwy
left a comment
There was a problem hiding this comment.
I wonder if we should cut this as a minor since it could affect existing users' muscle memory, and is sort of a feature-ish.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1122 +/- ##
=======================================
Coverage 81.05% 81.05%
=======================================
Files 54 54
Lines 2265 2265
Branches 684 683 -1
=======================================
Hits 1836 1836
Misses 424 424
Partials 5 5 ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
|
Hi! Is there any progress on this? We have s monorepo with ~ 50 packages and it is hard to look for a changed package everytime. This feature will be very handy |
Sorry, something went wrong.
|
I think it might be better to merge this in next altogether. In #1879 it'll already sort though |
Sorry, something went wrong.
|
Yeah, @Filipoliko - I'm sorry this got stale and didn't get in. As mentioned, #1879 revamps this area quite a bit and kinda includes this change anyway. So it's better for us to close this PR to avoid future conflicts and stuff. |
Sorry, something went wrong.
Resolves #1077