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

Use session classloader when loading deferred middleware #705

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

plexus
Copy link
Contributor

@plexus plexus commented Aug 31, 2021

When loading CIDER middleware via the sideloader we need to make sure that extra
namespaces that are loaded later are also requested via the sideloader. The
original sideloader already provided a macro for
this (with-session-classloader), we just need to use it. This does mean that now
all our middlewares need to depend on the session middleware.

(see an earlier comment hinting at the fact this would still need to happen: nrepl/nrepl#185 (comment))

Part of: clojure-emacs/cider#3037

  • The commits are consistent with our contribution guidelines
  • You've added tests to cover your change(s)
  • All tests are passing
  • The new code is not generating reflection warnings
  • You've updated the README (if adding/changing middleware)

@plexus
Copy link
Contributor Author

plexus commented Aug 31, 2021

I didn't add tests or docs yet for this one. wdyt @bbatsov, maybe not worth the trouble?

When loading CIDER middleware via the sideloader we need to make sure that extra
namespaces that are loaded later are also requested via the sideloader. The
original sideloader already provided a macro for
this (with-session-classloader), we just need to use it. This does mean that now
all our middlewares need to depend on the session middleware.
@plexus
Copy link
Contributor Author

plexus commented Aug 31, 2021

Not sure what's going on here

Using SSH Config Dir '/home/circleci/.ssh'
git version 2.11.0
Cloning git repository
Cloning into '.'...
Warning: Permanently added the RSA host key for IP address '140.82.114.4' to the list of known hosts.
Permission denied (publickey).
fatal: Could not read from remote repository.

@bbatsov
Copy link
Member

bbatsov commented Aug 31, 2021

No idea as well. I wonder if it's some temporary issue with Circle or our build isn't using some legacy Circle functionality.

@bbatsov
Copy link
Member

bbatsov commented Aug 31, 2021

I didn't add tests or docs yet for this one. wdyt @bbatsov, maybe not worth the trouble?

Probably we should update the docs, but we can do this in bulk at the end of the "project". Tests for this are probably an overkill. If the sideloading is working we can potentially dropped the deferred loading at some point, as we can load the middleware on demand via the sideloader, which would be nicer IMO.

@bbatsov bbatsov merged commit f3d3563 into clojure-emacs:master Sep 1, 2021
@plexus plexus deleted the use-session-classloader branch September 1, 2021 11:10
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