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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Clojure 1.11.1 #1039

Merged
merged 3 commits into from Dec 8, 2023
Merged

Update to Clojure 1.11.1 #1039

merged 3 commits into from Dec 8, 2023

Conversation

brandonvin
Copy link
Contributor

@brandonvin brandonvin commented Dec 6, 2023

#1038

Update Clojure version to 1.11.1 in project.clj.

Initially, this caused a test to fail:

Testing: riemann.test-test
Running tests in: riemann.test-test
  Running test: #'riemann.test-test/only-one-tap-per-context
    FAIL
      Expected: (re-find #"Tap :foo \(.+?:\) already defined at :" err)
      Actual: (not (re-find #"Tap :foo \(.+?:\) already defined at :" Unexpected error macroexpanding riemann.test/tap at (/tmp/form-init8155462538598659347.clj:1:6476).))
  Finished test: #'riemann.test-test/only-one-tap-per-context

The riemann.test/tap macro throws a RuntimeException if a tap with the given name already exists:

; Make sure no other tap conflicts
(when-let [extant (contains? @*taps* name)]
(throw (RuntimeException.
(str "Tap " name " (" file ":" line ")"
" already defined at "
(:file extant) ":" (:line extant)))))

Clojure changed how errors in macroexpansion are reported - it's now a CompilerException with the original RuntimeException as a cause, so the assertion needs to pull out the ex-cause.

I ran tests locally by mimicking the steps in .github/workflows/test.yaml. If there's a way to run the Gtihub test workflow on this PR, that would be great! 馃檹

Recent versions of Clojure changed to throw a CompilerException
if macroexpansion fails. The RuntimeException thrown by riemann.test/tap
is now the cause.
@smortex
Copy link
Contributor

smortex commented Dec 6, 2023

Dang, CI does not currently run on PRs. I opened #1040 to address this issue.

@jamtur01
Copy link
Member

jamtur01 commented Dec 7, 2023

I don't think I can manually trigger a GH action on the branch - if you push an update it should run.

@smortex
Copy link
Contributor

smortex commented Dec 7, 2023

I am not sure that closing / reopening the PR will do the trick (since the branch has no CI bits). I think that rebasing on top of main will do the trick:

git fetch origin       # Download the latest code we have here
git rebase origin/main # Move your commits on top of the main branch
git push -f            # Send the changes (-f is required because we re-wrote history)

Thanks!

@brandonvin
Copy link
Contributor Author

Thank you for the quick change to get CI running on PRs! That's great.

I synced my fork with this repo, and that has triggered CI on the branch.

Copy link
Contributor

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM! 馃挴

@jamtur01
Copy link
Member

jamtur01 commented Dec 8, 2023

And me! Thanks for this! 馃憤

@jamtur01 jamtur01 merged commit 40de2b6 into riemann:main Dec 8, 2023
7 checks passed
@brandonvin
Copy link
Contributor Author

Thank you @smortex and @jamtur01 for the quick feedback and help on this!

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

3 participants