-
-
Couldn't load subscription status.
- Fork 230
[wip] feat: uses latest version of depencies for jack-in #2960
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
Conversation
✅ Deploy Preview for calva-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
src/extension.ts
Outdated
| void depsClj | ||
| .downloadDepsClj(context.extensionPath) | ||
| .finally(() => { | ||
| void refreshJackInDependencyVersions(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added here the code to get latest versions, because of the fallback to use deps.clj
| const NREPL_VERSION = () => getConfig().jackInDependencyVersions['nrepl'], | ||
| CIDER_NREPL_VERSION = () => getConfig().jackInDependencyVersions['cider-nrepl'], | ||
| PIGGIEBACK_VERSION = () => getConfig().jackInDependencyVersions['cider/piggieback']; | ||
| const jackInVersionFor = (key: JackInDependencyKey) => getEffectiveJackInDependencyVersions()[key]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid get all config, and use directly the function that return the versions
| const depsCljJarPath = getDepsCljJarPath(); | ||
| if (depsCljJarPath) { | ||
| try { | ||
| const { stdout } = await execFileAsync('java', ['-jar', depsCljJarPath, ...args]); | ||
| const version = parseFindVersionsOutput(stdout); | ||
| if (version) { | ||
| return version; | ||
| } | ||
| errors.push(`deps.clj output missing version for ${library}`); | ||
| } catch (error) { | ||
| errors.push(`deps.clj failed: ${(error as Error).message}`); | ||
| } | ||
| } else { | ||
| errors.push('deps.clj.jar not available'); | ||
| } | ||
|
|
||
| throw new Error(errors.join(' | ')); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Calva already download deps.clj jar, added this fallback to try to get the versions if the first approach fails.
I don't know if is needed to be honest, adds more complexity and I can't argue if when it fails by any reason using clojure cli, if this will be a good fallback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is great. So if someone is using Leiningen and do not have clojure they will still be running with latest jack-in dependencies.
| 'nrepl': '1.3.1', | ||
| 'cider-nrepl': '0.55.4', | ||
| 'cider/piggieback': '0.6.0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These versions can be bumped to the latest (or any other specific version), before we merge the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which are the latest now? I think we should use latest. And in the changelog we can put the versions used in Calva currently, so that someone who wants to configure those can easily find them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PEZ the latest are:
cider-nrepl = '0.58.0'
cider/piggieback = '0.6.1'
nrepl = '1.5.1'
in the changelog we can put the versions used in Calva currently
So, do you want to bump the default versions to the latest and add to the changelog which versions? The ones that were before the bump? or the latest?
|
Thanks! This is so wonderful. And so wonderfully summarized. I will look closer tomorrow, late here now. =) |
|
The build is failing because of formatting btw. You can install the Prettier extension and it will run as a watcher if you are using the Build task. Then code will always be formatted on save. |
Co-authored-by: Peter Strömberg <pez@pezius.com>
|
Thanks. Looks fabulous! I've suggested an update to the changelog entry. Only other thing I think we want is that we log the checking to the console, also when things go well. Which versions we find and when global storage is updated. Good when developing on Calva and also for users to check when they report any problems. We could also log latest versions to Calva says, together with the effective version, and from where the effective versions are decided (like “latest known by Calva”, “user configured override”, "Calva defaults fallback”). I love the integration tests! ❤️ |
Co-authored-by: Peter Strömberg <pez@pezius.com>
|
@PEZ added the logs and messages to Calva says |
|
Sweet! |
What has changed?
nREPL,cider-nrepl, andpiggiebackversions on first startup (via clojure/deps; falls back to deps.clj), caches them in global state, and falls back to built-in defaults if fetching fails.src/nrepl/jack-in-dependency-versions.tsimplements fetching, EDN parsing, persistence, and a single-flight refresh to avoid duplicate requests.jackInVersionForhelper inproject-typesand removed direct dependency on raw config for version lookup.deps.cljis downloaded; this is non-blocking for startup.Fixes #2959
My Calva PR Checklist
I have:
devbranch. (Or have specific reasons to target some other branch.)published. (Sorry for the nagging.)[Unreleased]entry inCHANGELOG.md, linking the issue(s) that the PR is addressing.npm run prettier-format)npm run eslintbefore creating your PR, or runnpm run eslint-watchto eslint as you go).