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

Update nREPL to 0.4.4 #703

Merged
merged 1 commit into from Sep 14, 2018
Merged

Update nREPL to 0.4.4 #703

merged 1 commit into from Sep 14, 2018

Conversation

@bbatsov
Copy link
Contributor

bbatsov commented Jun 26, 2018

As no one really commented on #695 I've decided to try an alternative approach to get some conversation going. Here's my preferred (quick) solution to the migration to the new nREPL. I know this is probably less than ideal, but it's a one time deal, and I'd really appreciate your support as I rather be dealing with nREPL improvements than with migration paths.

nREPL was recently moved out of clojure-contrib and its development
continued under a dedicated org (https://github.com/nrepl). The new project is (and will remain
to be) completely backwards compatible on the nREPL API level, but
the namespace had to be changed.

This fixes #695.

@alandipert

This comment has been minimized.

Copy link
Contributor

alandipert commented Jun 29, 2018

Thanks @bbatsov, if this is a backward compatible change I can't see any reason not to do it.

@alandipert

This comment has been minimized.

Copy link
Contributor

alandipert commented Jun 29, 2018

Oh, this could use a CHANGELOG.md entry though. Thanks in advance for adding one that summarizes the change.

@arichiardi

This comment has been minimized.

Copy link
Contributor

arichiardi commented Jun 29, 2018

Just for future reference, this is not really backward compatible, see:
https://github.com/nrepl/nREPL#status

However the tooling goal would be to move towards it we could bump boot's major and advertise the change way in advance. In the meantime folks (like myself) can try things out with all the master code bases (many involved) and confirm everything works.

@bbatsov

This comment has been minimized.

Copy link
Contributor Author

bbatsov commented Jun 29, 2018

Thanks @bbatsov, if this is a backward compatible change I can't see any reason not to do it.

The protocol is completely the same, but the project has a new namespace, which means that middleware will need small tweaks to work with it. That being said - I'm the direct author/maintainer of almost all popular nREPL middleware and I do plan to switch them to target the new nREPL anyways. :-)

Oh, this could use a CHANGELOG.md entry though. Thanks in advance for adding one that summarizes the change.

Of course. I'll add this in a bit.

@bbatsov bbatsov force-pushed the bbatsov:new-nrepl branch from bbbbddc to 474c4bf Jun 29, 2018
@bbatsov

This comment has been minimized.

Copy link
Contributor Author

bbatsov commented Jun 29, 2018

Changelog entry added.

@bbatsov bbatsov mentioned this pull request Jun 29, 2018
@bbatsov bbatsov force-pushed the bbatsov:new-nrepl branch 2 times, most recently from 9a992b0 to 679f8d3 Jul 26, 2018
@bbatsov bbatsov changed the title Update nREPL to 0.4.1 Update nREPL to 0.4.3 Jul 26, 2018
@bbatsov

This comment has been minimized.

Copy link
Contributor Author

bbatsov commented Jul 26, 2018

PR updated for nREPL 0.4.3.

CHANGES.md Outdated
@@ -8,6 +8,16 @@

[702]: https://github.com/boot-clj/boot/pull/702

#### Breaking

Updated nREPL to 0.4.3. The new nREPL has a new artefact coordinates

This comment has been minimized.

Copy link
@thenonameguy

thenonameguy Jul 26, 2018

s/artefact/artifact ?

This comment has been minimized.

Copy link
@bbatsov

bbatsov Jul 26, 2018

Author Contributor

Not in my part of the world. 😆

This comment has been minimized.

This comment has been minimized.

Copy link
@bbatsov

bbatsov Jul 26, 2018

Author Contributor

Yeah, yeah. I know everyone in programming uses American English spellings, but it's hard for me to remember this all the time. Especially when my spell checker highlights artifact as incorrect. :-)

@bbatsov

This comment has been minimized.

Copy link
Contributor Author

bbatsov commented Aug 7, 2018

The build failure is unrelated, btw.

@alandipert Can you tell me if you want us to move forward with this or not? I've invested a tremendous amount of time already coordinating the nREPL upgrade across all related projects and that's the final missing piece. I'd be extremely happy if we can just wrap this up and I can focus on actually improving the server and its middleware. :-)

nREPL was recently moved out of clojure-contrib and it's developed
continued under a dedicated org. The new project is (and will remain
to be) completely backwards compatible on the nREPL API level, but
the namespace had to be changed.
@bbatsov bbatsov force-pushed the bbatsov:new-nrepl branch from 679f8d3 to e3e69d7 Aug 7, 2018
@bbatsov bbatsov changed the title Update nREPL to 0.4.3 Update nREPL to 0.4.4 Aug 7, 2018
@alandipert alandipert merged commit e3e69d7 into boot-clj:master Sep 14, 2018
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@bbatsov

This comment has been minimized.

Copy link
Contributor Author

bbatsov commented Sep 15, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.