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

Dependency versions can be pinned #228

Merged
merged 18 commits into from
Jun 13, 2024
Merged

Conversation

teodorlu
Copy link
Contributor

@teodorlu teodorlu commented Jun 12, 2024

Please answer the following questions and leave the below in as part of your PR.

Progress

I'm ready to merge if everything looks good.

Motivation

neil users may not always want to update a dependency, for example due to introduced breaking changes. Currently, neil dep upgrade will upgrade all those dependencies, and the user will have to revert the change by hand.

Implemented behavior

Dependencies in deps.edn can now be pinned. Pinned dependencies have :neil/pinned set to true. Running neil dep upgrade on the following deps.edn file will upgrade cheshire/cheshire, but leave hiccup/hiccup alone.

{:deps {hiccup/hiccup {:mvn/version "1.0.0"
                       :neil/pinned true}
        cheshire/cheshire {:mvn/version "4.0.0"}}
 :aliases {}}

Pinned dependencies are ignored when neil dep upgrade updates dependencies.

Future ideas

neil dep add could support an optional --pin argument. Something like this:

neil dep add hiccup --pin

@teodorlu teodorlu marked this pull request as ready for review June 12, 2024 13:22
Commands such as

    neil-dev dep add hiccup/hiccup --version 1.0.1 --pin false

now actually work. Previously, --pin false was a noop, nil and false was
treated as the same value.

New behavior:

    --pin true sets :neil/pinnned to true
    --pin false sets :neil/pinned to false
    no --pin argument does not change the file.
@borkdude
Copy link
Contributor

I think this PR takes the original issue a bit further than I imagined. Just supporting adding :neil/pinned true manually in deps.edn is basically what I imagined. I don't think I'd want to type neil dep pin foobar/foobar which is about the same, if not more effort than just editing the deps.edn.

@teodorlu
Copy link
Contributor Author

teodorlu commented Jun 12, 2024

OK, I'll update the PR.

I agree that it might be too much to introduce at once, at least before we know whether a new option and changed neil dep add behavior is actually needed.

@teodorlu
Copy link
Contributor Author

I don't think I'd want to type neil dep pin foobar/foobar which is about the same, if not more effort than just editing the deps.edn.

Note: the behavior change i implemented did not add a new subcommand, but an optional argument :pin to neil dep add. Similar, but not exactly the same.

@teodorlu
Copy link
Contributor Author

@borkdude I think this looks good now–mind giving a second review?

@borkdude
Copy link
Contributor

borkdude commented Jun 12, 2024 via email

@borkdude borkdude merged commit a38be9f into babashka:main Jun 13, 2024
4 checks passed
@teodorlu teodorlu deleted the neil-dep-pin branch June 13, 2024 15:49
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.

2 participants