Skip to content
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

Support for all argument in unlock #2624

Merged
merged 6 commits into from
Nov 26, 2021
Merged

Conversation

joshrotenberg
Copy link
Contributor

@joshrotenberg joshrotenberg commented Oct 3, 2021

This alters the behavior of the unlock command as outlined in #2583. Previously, calling rebar3 unlock with no further arguments would simply delete the rebar.lock file. This change requires the --all flag to unlock all dependencies, and makes calling unlock with no arguments an error. When specifying a list of dependencies to unlock, the behavior is unchanged.

#2583 requests similar behavior for upgrade and plugins upgrade. These will be addressed in separate PRs.

@ferd
Copy link
Collaborator

ferd commented Oct 25, 2021

Sorry for the late review. I think this is a fine implementation.

We should probably test the failing case, which wouldn't be too hard, but let me know if you need help there.

I'm fine taking PRs for plugin upgrades and the lib upgrades, and I'm also good with having them part of this PR. In all cases, I would wait to merge them together to make sure they're all happening in the same future release and don't end up inconsistent across versions.

@joshrotenberg
Copy link
Contributor Author

Sorry for the late review. I think this is a fine implementation.

No problem, good to know it looks ok.

We should probably test the failing case, which wouldn't be too hard, but let me know if you need help there.

I may need help, I was having some trouble figuring out if I'm surfacing the error correctly because catching it in the tests was a struggle. I'll get back to it and let you know if I need help, thank you!

I'm fine taking PRs for plugin upgrades and the lib upgrades, and I'm also good with having them part of this PR. In all cases, I would wait to merge them together to make sure they're all happening in the same future release and don't end up inconsistent across versions.

I think I will split them up into separate PRs since I think they are all relatively independent.

@joshrotenberg joshrotenberg marked this pull request as ready for review October 26, 2021 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants