-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Sort workspace dependencies #55
base: main
Are you sure you want to change the base?
Conversation
arr.sort(); | ||
} | ||
Item::Table(table) => { | ||
sort_table(table, group); |
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.
I verified that if I leave out this new line of code (sort_table
), the new test fails - so the test is covering this correctly.
Thanks for the PR!! I will check this out tomorrow and merge it then. A quick look over it seems like everything is good! Also thanks for adding a test for this 👍 |
Just noting this doesnt help with workspace members which are using inherited dependencies, which is broadly covered in #47 |
any updates on this? would be great to get in :) |
Any progress on merging this? Would be nice to have cargo sort working with this common syntax. |
@DevinR528 It's been a pretty long day :), do you need any help to get this merged? |
cargo-sort had problems parsing workspace dependencies, looks like it's unmaintained. Fortunately someone made a fix in DevinR528/cargo-sort#55. The branch was cloned and diff examined, we can use for now until we can invest more time to find and configure a replacement. Signed-off-by: Marcel Guzik <[email protected]>
@DevinR528 can we get this merged soon? Would be a great feature to have :) |
@DevinR528 hope everything is alright, can this be merged please? |
@DevinR528 Just bumping this again. |
Hey @nipunn1313 do you mind rebasing this, pretty please? |
Part of the new workspace dependencies feature described here https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#inheriting-a-dependency-from-a-workspace Fixes DevinR528#54
8606671
to
425adb0
Compare
Sure - done. |
ping @DevinR528 , can we get this merged please |
Hi @DevinR528 , Kindly merge this PR |
Hi @jplatte what's your thoughts on this? |
Hey, sorry for the long time of silence. Unfortunately I can't promise to get to this PR within any given timeframe as I only took over maintenance because I use this in another project and Devin trusted me to do some maintenance here. I'm not deeply familiar with the project's internals and have a lot of higher priority projects as well on my plate, and some rather stressful changes in my life outside programming. I will keep this in my pending GH notifications until I can get to it though. |
Part of the new workspace dependencies feature described here https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#inheriting-a-dependency-from-a-workspace
Fixes #54