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

Updated version.clj #395

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Updated version.clj #395

merged 2 commits into from
Jul 11, 2023

Conversation

dotemacs
Copy link
Contributor

@dotemacs dotemacs commented Jul 11, 2023

  • The commits are consistent with our contribution guidelines
    - [ ] You've added tests (if possible) to cover your change(s)
  • All tests are passing (run lein do clean, test)
  • [] Code inlining with mranderson works and tests pass with inlined code (run ./build.sh install -- takes a long time)
    - [ ] You've updated the changelog (if adding/changing user-visible functionality)
    - [ ] You've updated the readme (if adding/changing user-visible functionality)

This is in response to: #393 (comment)
and this comment in particular:

version-clj makes me nervous as it went 1.x -> 2.x with no changelog.

I guess not everybody uses CHANGELOGs...

The project, version-clj, was on version 1.0.0 before update to 2.0.0.

It only uses one function from version-clj in
refactor-nrepl.artifacts/artifact-versions: version-sort:

This is the change that went into that function from version 1.0.0 to 2.0.0:

xsc/version-clj@v1.0.0...v2.0.0#diff-c6b8870bce46e7fe981de278c5a5eb53847fd489211487800632111290202b63R56

No change to the actual function, but that function uses version-compare function, which did have a change, see:

xsc/version-clj@v1.0.0...v2.0.0#diff-c6b8870bce46e7fe981de278c5a5eb53847fd489211487800632111290202b63R23

If you look at this commit:
xsc/version-clj@c05eeb8 you'll see that the tests for version-sort & version-compare were added for the release 1.0.0 and were not amended at the latest release 2.0.2.

And finally a pedantic comparison of version 1.0.0 & the current stable release 2.0.2 (which is this commit's update):

xsc/version-clj@v1.0.0...v2.0.2

The project was on version 1.0.0 before update to 2.0.0.

It only uses one function from version-clj in
refactor-nrepl.artifacts/artifact-versions: version-sort.

This is the change that went into that function from version 1.0.0 to
2.0.0:

xsc/version-clj@v1.0.0...v2.0.0#diff-c6b8870bce46e7fe981de278c5a5eb53847fd489211487800632111290202b63R56

No change to the actual function, but that function uses
version-compare function, which did have a change, see:

xsc/version-clj@v1.0.0...v2.0.0#diff-c6b8870bce46e7fe981de278c5a5eb53847fd489211487800632111290202b63R23

If you look at this commit:
xsc/version-clj@c05eeb8
you'll see that the tests for version-sort & version-compare were
added for the release 1.0.0 and were not amended at the latest release
2.0.2.

And finally a pedantic comparison of version 1.0.0 & the current
stable release 2.0.2 (which is this commit's update):

xsc/version-clj@v1.0.0...v2.0.2
@dotemacs
Copy link
Contributor Author

dotemacs commented Jul 11, 2023

Running lein do clean, test locally, the tests pass.

As well as 7/13 CI jobs.

Can you please re-run the tests on all the CI profiles, to ensure that it's not just a glitch?

Thanks

Copy link
Member

@vemv vemv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for another careful analysis!

Between it, and the fact that defn artifact-versions has specific test coverage, we are in the safe zone.

Please tweak the CHANGELOG.

@vemv
Copy link
Member

vemv commented Jul 11, 2023

Cheers 🍻

@vemv vemv merged commit 0a9a6ee into clojure-emacs:master Jul 11, 2023
@dotemacs dotemacs deleted the update-version-clj branch July 11, 2023 20:11
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