-
Notifications
You must be signed in to change notification settings - Fork 19
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
trim deps and upgrade to @jupyterlab 3.6 #137
Conversation
Hi @tavin, thank you for persevering with the various PRs. I'll sort through them today! This PR I don't think we can merge; we need those deps, as they're imported in various modules. I'll close this PR, and make another pass of our other PRs! |
Hi @agoose77 I think whether you want to list those deps in Of course that's apart from |
My take is slightly different — if we use a package, we should declare a dependency upon it, otherwise we depend upon our other dependencies to do so. One can end up in a situation (although fairly unlikely in this case) where everyone assumes their dependencies do the same until someone drops it and breaks. |
@agoose77 I'm 100% happy to agree with you and keep them all in The importance of this PR lies in keeping the yarn lockfile honest. We're both trying to add features for which an upgrade to v3.6 is at least beneficial (whether it's technically necessary in your case, I'm not sure, but it's happening unless you pin some deps). |
Yes, agreed, and I think it's quite possible that the lockfile was out-of-sync in the past! Concerning updating to 3.6, that's a decision that we need to think carefully about; it will mean bumping our minimum JupyterLab version to 3.6, which is the newest minor version. Therefore, I'd prefer to hold off and implement workarounds until that time (which is unfortunate). My plan is to merge #139, which upgrades our lower bounds to the Python package version, and update #102 to add the |
In that case please consider agoose77#2 -- which is totally out of sync now, but contains this commit. |
What's the motivation behind this commit? In general, resolutions are used for fixing the metadata of dependencies. In this case, it would force yarn to always give 3.5.1, i.e. the same as writing "dependencies": {
"@jupyterlab/application": "3.5.1",
...
} |
The motivation is to fix the yarn lockfile. The resolutions could be removed later. |
OK. It's my understanding that we shouldn't need |
@agoose77 what happened on your branch looks a bit like the yarn lockfile was deleted and rebuilt (I couldn't otherwise account for some of the version bumps). Also because the markdownviewer was new, I was just trying to keep the |
Hi @rowanc1 @agoose77 maybe this will be of some help.
I know #115 will be simplified by this, and it should clear up some dep/build uncertainties in #102.
Cheers.