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

Ignore the patch version of cider-nrepl when doing a required version check #2705

Closed
bbatsov opened this issue Sep 4, 2019 · 3 comments · Fixed by #2708
Closed

Ignore the patch version of cider-nrepl when doing a required version check #2705

bbatsov opened this issue Sep 4, 2019 · 3 comments · Fixed by #2708
Labels
enhancement good first issue A simple tasks suitable for first-time contributors

Comments

@bbatsov
Copy link
Member

bbatsov commented Sep 4, 2019

Today you'd get a warning if cider-required-middleware-version is 0.22.1 and you're on cider-nrepl 0.22.2. I think it'd be best if we just ignored the patch segment of the version while doing this comparison.

@bbatsov bbatsov added enhancement good first issue A simple tasks suitable for first-time contributors labels Sep 4, 2019
@jgerman
Copy link
Contributor

jgerman commented Sep 11, 2019

This is my first time looking at the Cider code (new to elisp as well) but I think this is pretty straightforward.

If the patch is dropped from cider-required-middleware-version:

(defconst cider-required-middleware-version "0.22")

and use string-prefix-p in the check:

 ((not (string-prefix-p cider-required-middleware-version middleware-version))
      (cider-emit-manual-warning "troubleshooting.html#_cider_complains_of_the_cider_nrepl_version"
                                 "CIDER %s requires cider-nrepl %s+, but you're currently using cider-nrepl %s. The version mismatch might break some functionality!"
                                 cider-version cider-required-middleware-version middleware-version)))))

If that's acceptable I can put together a PR.

@bbatsov
Copy link
Member Author

bbatsov commented Sep 11, 2019

It can't be done this way, as we also use the same version number to tell CIDER which cider-nrepl to load. It'd be adjust only the code that handles the warning. Emacs has a bunch of functions for comparing versions that will help with this.

@jgerman
Copy link
Contributor

jgerman commented Sep 11, 2019

I ran into that issue immediately after posting this when I tried to cider-jack-in instead of cider-connect. :)

I'll dig a little deeper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue A simple tasks suitable for first-time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants