clojure-emacs / cider-nrepl

A collection of nREPL middleware to enhance Clojure editors with common functionality like definition lookup, code completion, etc.
https://docs.cider.mx/cider-nrepl
670 stars 175 forks source link

Bump Compliment #837

Closed alexander-yakushev closed 5 months ago

bbatsov commented 7 months ago

Please, mention this in the changelog as well.

vemv commented 7 months ago

I had it pending to try this, sorry about that!

I'd like to give it a few days usage, now for real.

https://github.com/clojure-emacs/cider-nrepl/issues/836 is a precondition as well

bbatsov commented 6 months ago

I guess now we can continue with this PR.

alexander-yakushev commented 6 months ago

Managed to somehow accidentally close it after rebasing.

alexander-yakushev commented 6 months ago

Tests seem to be failing semi-randomly, does anyone have any immediate ideas? Or should I investigate case by case?

vemv commented 6 months ago

It's always complete-test, right?

I'd appreciate if you could give it a shot, however don't hesitate pinging for a second pair of eyes

alexander-yakushev commented 6 months ago

I'm an idiot. Didn't know that cider-nrepl specifies completion sources explicitly.

alexander-yakushev commented 5 months ago

1.9 stuff still fails. Does everybody agree that cider-nrepl should get the same treatment as nREPL testing matrix-wise? https://github.com/nrepl/nrepl/pull/305

vemv commented 5 months ago

Normally we'd say "not yet" but recently we discovered that 1.9 was unawarely broken:

https://github.com/clojure-emacs/cider-nrepl/pull/840

So I'd vote for dropping it again 👍

alexander-yakushev commented 5 months ago

I took the liberty of refactoring CI config to remove the excludes, and instead cover the correct matrix cells with more sets. WDYT?

alexander-yakushev commented 5 months ago

Normally we'd say "not yet" but recently we discovered that 1.9 was unawarely broken

Besides, it makes sense that cider-nrepl compatibility is equal or a subset the nREPL one, not a superset.

alexander-yakushev commented 5 months ago

I almost succeeded, but tests for JDK8 and legacy parser still fail. I would need some help with this one.

vemv commented 5 months ago

While the refactoring is certainly welcome, I'd appreciate if the changes merely removed the 1.8 and 1.9 from the excludes matrix.

That way we can be sure that the failures don't have to do with the refactoring itself.

You can propose the refactoring afterwards (or just now, in an isolated PR)

Thanks!

bbatsov commented 5 months ago

I took a quick look at the failing tests, and I'm puzzled by them, as they haven't been changed in a while and don't seem to have any relation to completion. I don't think they haven't anything to do with the parser version either.

vemv commented 5 months ago

Can second that, but master is green and hasn't shown flakiness in that particular area.

vemv commented 5 months ago

I'm working on merging this as we speak

vemv commented 5 months ago

https://github.com/clojure-emacs/cider-nrepl/commit/5afa8c6afd03d5a94f661eb00d849063d72d8efc

Thanks for your work!

vemv commented 5 months ago

https://github.com/clojure-emacs/cider/releases/tag/v1.13.0 https://github.com/clojure-emacs/cider-nrepl/blob/master/CHANGELOG.md#0450-2024-01-14

alexander-yakushev commented 5 months ago

Awesome, thanks!