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

Add support for JupyterLab 4.x #34

Merged
merged 27 commits into from
Mar 5, 2024
Merged

Conversation

isumitjha
Copy link
Contributor

@isumitjha isumitjha commented Jan 2, 2024

Fixes #29 and #33

I have been working on this lately, out of the desire to see this working on JupyterLab 4.x.
To be able to make sure that it really works, I tested it locally and also fixed the integration tests

You can see the CI is green in the Actions for my fork here:
https://github.com/isumitjha/jupyterlab-conda-store/actions/runs/7389583334

This is what it accomplishes:

  • Upgrade to JupyterLab 4.x
  • Fix integration Tests
  • Execute integration tests in the CI
  • Add ui-tests to workspace to be able to run them easily
  • Fix extension code to work on JupyterLab 4.x
  • Update jupyter server test config to be able to run playwright tests, which wasn't working earlier.
  • Fix Typescript formatting/prettier
  • Add/update docs

ping @trallard

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

Additional information

@isumitjha
Copy link
Contributor Author

Working example:

conda-store-screencast.mp4

@isumitjha isumitjha mentioned this pull request Jan 2, 2024
@pavithraes pavithraes added type: enhancement 💅🏼 area: dependencies 📦 Issues related to conda-store dependencies labels Jan 4, 2024
@pavithraes pavithraes self-requested a review January 4, 2024 12:16
@pavithraes
Copy link
Member

Thank you so much for this PR, we've wanted to do this for a while. We'll prioritize getting this reviewed and merged soon 🌻


Aside,

I know this is a little late, but ✨ welcome! ✨ We really appreciate your contributions across the conda-store repos!

It'll also great if you could drop a comment on any issue you're interested in working on in the future - that way we can provide support, share any additional context, and plan for code reviews early. :)

@isumitjha
Copy link
Contributor Author

Thanks @pavithraes glad to contribute.

It'll also great if you could drop a comment on any issue you're interested in working on in the future - that way we can provide support, share any additional context, and plan for code reviews early. :)

Sure, will do that in future. I didn't initially want to commit before trying this one as I was not sure how much time it will take and when I started working on it I got stuck in the rabbit-hole of issues and eventually ended up creating the PR :)

One feedback I have is I had some issues finding the docs (couldn't find any), but looks like its probably under-construction: https://conda.store/jupyterlab-conda-store/introduction

@isumitjha
Copy link
Contributor Author

Hi! @pavithraes just wanted to check-in if there is anything I can do to help with the review here? :)

@nkaretnikov nkaretnikov removed their assignment Mar 2, 2024
@nkaretnikov nkaretnikov removed their request for review March 2, 2024 18:35
@nkaretnikov
Copy link
Contributor

We had a meeting about this PR last week. IIUC @krassowski should help review this one.

@krassowski
Copy link
Contributor

Yes, it's on my to-do list (I was off sick last couple of days, but will get to this ASAP).

Copy link
Contributor

@krassowski krassowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general, some tiny suggestions.

.yarnrc.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@isumitjha
Copy link
Contributor Author

Thanks for the review @krassowski I have addressed all the comments, let me know if it looks in good shape now.

Copy link
Contributor

@krassowski krassowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @isumitjha, this looks good to me!

For maintainers: tested locally and reviewed the code base. It would be relatively easy to make the extension support both Lab 3.x and 4.x (there is only one code change) but JupyterLab 3.x is reaching end of maintenance soon so I personally don't think it is necessary.

@@ -17,15 +17,15 @@
A JupyterLab extension for [conda-store][conda-store-repo].

> [!NOTE]
> Currently, this extension is only compatible with JupyterLab `>= 3.0 <= 4.0`.
> This extension is only compatible with JupyterLab `>= 4.0`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To maintainers: after releasing a new version I would update this to say something like "Version YYYY.MM.X+ is only compatible with JupyterLab `>= 4.0, use version YYYY.MM.Y for older JupyterLab".

@pavithraes
Copy link
Member

Thank you, @isumitjha and @krassowski! ✨

Since Mike has ✅ this, I'll go ahead and merge. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dependencies 📦 Issues related to conda-store dependencies type: enhancement 💅🏼
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

Support for JupyterLab 4
6 participants