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

Add handling for java version strings with $OPT chunks #82

Merged
merged 5 commits into from
Feb 12, 2020

Conversation

tstirrat15
Copy link
Contributor

Motivation

See comment here: #57 (comment)

It's currently causing cider to fail for me when I jack in, and the traceback includes orchard failing to get the correct version string.

Changes

  • Add a test for a java version string with a $OPT chunk
  • Fix the test by lopping off the $OPT in the let clause of parse-java-version

Notes

I'm still pretty new to clojure, so if there's a more sane/elegant/appropriate way to lop off the $OPT bit, please let me know.

@@ -92,7 +92,8 @@
"Parse a Java version string according to JEP 223 and return the appropriate version."
[java-ver]
(try
(let [[major minor _] (str/split java-ver #"\.")
(let [[no-opt _] (str/split java-ver #"-")
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably add some comment here explaining the need for this, as it's exactly obvious.

@bbatsov
Copy link
Member

bbatsov commented Jan 31, 2020

The change seems fine to me. It should also be reflected in the changelog as a bugfix.

@tstirrat15
Copy link
Contributor Author

I've added those changes. Let me know if you want me to squash commits or anything like that.

@tstirrat15
Copy link
Contributor Author

What's the status?

@bbatsov bbatsov merged commit 63f5cbc into clojure-emacs:master Feb 12, 2020
@bbatsov
Copy link
Member

bbatsov commented Feb 12, 2020

My bad. I thought I had already merged this! Thanks for tackling it. Next week I'll cut some new Orchard release, as this week I'm busy around IN/Clojure.

@tstirrat15
Copy link
Contributor Author

Sounds good. Thank you!

Where should I look for a notification of that new release?

@bbatsov
Copy link
Member

bbatsov commented Feb 13, 2020

I think that if you're watching a repo you get release notifications. I don't usually do public release announcements for bugfix releases.

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