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

Don't drop :exclusions when running neil dep add or neil dep upgrade #190

Merged
merged 12 commits into from
Sep 26, 2023

Conversation

teodorlu
Copy link
Contributor

@teodorlu teodorlu commented Sep 23, 2023

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

@teodorlu teodorlu marked this pull request as draft September 23, 2023 14:19
@teodorlu
Copy link
Contributor Author

teodorlu commented Sep 23, 2023

This is the test that fails:

https://github.com/teodorlu/neil/blob/ab7e5ffa92bbdb3dcbeac26a4fe6da985487faa6/test/babashka/neil/dep_upgrade_test.clj#L133

A deps.edn file that had a :sha version keeps that version around (per ab7e5ff). Example:

$ cat deps.edn 
{:deps {clj-kondo/clj-kondo
                            {:git/url "https://github.com/clj-kondo/clj-kondo"
                             :sha "6ffc3934cb83d2c4fff16d84198c73b40cd8a078"}}}
$ neil-dev dep upgrade
:action "upgrading" :lib clj-kondo/clj-kondo :current-version nil :sha 8d1531b23485815ab26a7da10eac9950ecc2338d
$ cat deps.edn 
{:deps {clj-kondo/clj-kondo
                            {:git/url "https://github.com/clj-kondo/clj-kondo"
                             :sha "6ffc3934cb83d2c4fff16d84198c73b40cd8a078"
                             :git/sha "8d1531b23485815ab26a7da10eac9950ecc2338d"}}}

bad!
This even causes clj to error:

$ clj
Error building classpath. git coord has both :sha and :git/sha for clj-kondo/clj-kondo

@teodorlu teodorlu marked this pull request as ready for review September 23, 2023 14:56
@teodorlu
Copy link
Contributor Author

teodorlu commented Sep 23, 2023

How about using reference-style markdown links for CHANGELOG.md?

Currently, we write the CHANGELOG like this (scroll right):

- [#181](https://github.com/babashka/neil/issues/181): fix `neil --version`
- fix tests by referring to latest hiccup ([@teodorlu](https://github.com/teodorlu))
- [#180](https://github.com/babashka/neil/issues/180): `neil dep upgrade`: allow upgrading from an unstable version to the latest unstable version ([@teodorlu](https://github.com/teodorlu))
- [#180](https://github.com/babashka/neil/issues/180): `neil dep upgrade`: with `--unstable`, opt-into unstable library updates ([@teodorlu](https://github.com/teodorlu))
- [#183](https://github.com/babashka/neil/issues/183): Don't drop `:exclusions` when running `neil dep add` or `neil dep upgrade` ([@borkdude](https://github.com/borkdude) and [@teodorlu](https://github.com/teodorlu))

With the proposed link format, we'd write the CHANGELOG like this (scroll right):

- [#181](https://github.com/babashka/neil/issues/181): fix `neil --version`
- fix tests by referring to latest hiccup ([@teodorlu])
- [#180](https://github.com/babashka/neil/issues/180): `neil dep upgrade`: allow upgrading from an unstable version to the latest unstable version ([@teodorlu])
- [#180](https://github.com/babashka/neil/issues/180): `neil dep upgrade`: with `--unstable`, opt-into unstable library updates ([@teodorlu])
- [#183](https://github.com/babashka/neil/issues/183): Don't drop `:exclusions` when running `neil dep add` or `neil dep upgrade` ([@borkdude] and [@teodorlu])

[@borkdude]: https://github.com/borkdude
[@teodorlu]: https://github.com/teodorlu

That way, we can write [@borkdude] and have the link to https://github.com/borkdude/ work automatically. Thoughts?

@borkdude
Copy link
Contributor

I'm using script/changelog.clj to render these links after typing, but if your suggested way renders the exact same way, I'd be down with that.

@borkdude borkdude merged commit 89bedc7 into babashka:main Sep 26, 2023
4 checks passed
@borkdude
Copy link
Contributor

Thanks a lot!

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