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

Upgrade extension to JupyterLab 4 #145

Merged
merged 19 commits into from
Aug 2, 2023

Conversation

mahendrapaipuri
Copy link
Contributor

@mahendrapaipuri mahendrapaipuri commented Jul 18, 2023

Following major changes have been made:

  • Migrated to hatch build system
  • Modernised async related stuff in handler
  • Bumped extension's version to 4.0.0
  • Migrated existing tslint.json into eslintrc.js using tslint-to-eslint-config
  • Added basic CI workflows

Closes #144

Use npm_builder hook to build labextension.

Use nodejs version hook to manage extension's version
Migrate deprecated tslint.json to eslintrc.js.

Update target in tsconfig to es2018

Lint ts src code and make changes for the src to be compatible for
Jlab 4
@welcome
Copy link

welcome bot commented Jul 18, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch mahendrapaipuri/jupyterlab-github/upgrade_to_jlab_4

@jtpio
Copy link
Member

jtpio commented Aug 1, 2023

Thanks @mahendrapaipuri for working on this!

Looks like CI was not triggered, not sure why but this also seems to be the case of the recent commits on master.

@mahendrapaipuri
Copy link
Contributor Author

mahendrapaipuri commented Aug 1, 2023

@jtpio You are welcome.

If we look into CI workflows, there are no "actual" workflows perse. The workflow is to create a link to binder on source branch of the PR and add it to comments.

Edit: binder config files are up to date as well

@jtpio
Copy link
Member

jtpio commented Aug 1, 2023

Ah ok thanks!

Then maybe we should add the base workflows from the extension template? https://github.com/jupyterlab/extension-template

@mahendrapaipuri
Copy link
Contributor Author

Fair enough. Makes sense. Do you think we can add workflows it in the current PR or create a new one?

@jtpio
Copy link
Member

jtpio commented Aug 1, 2023

Here would be fine, hopefully the workflows will be triggered properly.

@mahendrapaipuri
Copy link
Contributor Author

@jtpio Added basic CI workflows based on what we did for topbar extensions. For release management, we will integrate it with jupyter releaser as well?

@jtpio
Copy link
Member

jtpio commented Aug 1, 2023

Yes. Normally the extension template is already configured to be compatible with the releaser.

@jtpio
Copy link
Member

jtpio commented Aug 1, 2023

Yes. Normally the extension template is already configured to be compatible with the releaser.

Part of supporting the Jupyter Releaser would be to add the check-releaser workflow: https://github.com/jupyterlab/extension-template/blob/main/template/.github/workflows/check-release.yml.jinja

@mahendrapaipuri
Copy link
Contributor Author

@jtpio So, I was checking the check-release.yml workflow that you mentioned. If we look into the template, the check-release action it is attempting to use does not exist on jupyter_releaser actions. I was trying to understand what exactly the action check-release will do but could not find any.

So, the idea is that you will manage the releases via a forked jupyter_releaser repo or you prefer to add all release related workflows (https://github.com/jupyter-server/jupyter_releaser/tree/main/example-workflows) in the repo?

@jtpio
Copy link
Member

jtpio commented Aug 2, 2023

If we look into the template, the check-release action it is attempting to use does not exist on jupyter_releaser actions.

I think it corresponds to this one: https://github.com/jupyter-server/jupyter_releaser/blob/main/.github/actions/check-release/action.yml

So, the idea is that you will manage the releases via a forked jupyter_releaser repo or you prefer to add all release related workflows (https://github.com/jupyter-server/jupyter_releaser/tree/main/example-workflows) in the repo?

The recommended and most convenient way now is to add the workflows to the repo directly to avoid having to maintain the fork. This is used in the JupyterLab repo for example (and they are also included in the extension template):

The ones that are taken from jupyter_releaser repo

- name: Install Dependencies
run: |
pip install -e .
Copy link
Member

Choose a reason for hiding this comment

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

Normally we shouldn't need this as it might add side-effects, and this step does not exist during the actual release process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Maybe we should remove it from the extension-template repo as well?

Copy link
Member

Choose a reason for hiding this comment

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

Ah if it's there then yes 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is in the template. I have not changed anything from the template except replacing Jinja2 vars.

Copy link
Member

Choose a reason for hiding this comment

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

@mahendrapaipuri
Copy link
Contributor Author

Seems like check release workflow failed due to missing CHANGELOG.md file. From the docs of jupyter_releaser, it seems like the presence of CHANGELOG.md file is not a prerequisite.

Do you think jupyter_releaser should create one if it does not exist?

@jtpio
Copy link
Member

jtpio commented Aug 2, 2023

We can add an empty CHANGELOG.md for example taken from the template: https://github.com/jupyterlab/extension-template/blob/main/template/CHANGELOG.md

It is needed for check release workflow to generate a draft changelog.
The workflow expects the file to exist.
@jtpio
Copy link
Member

jtpio commented Aug 2, 2023

empty

We could also bootstrap a changelog using github-activity and the --all flag: https://github-activity.readthedocs.io/en/latest/index.html#generate-a-markdown-changelog

@mahendrapaipuri
Copy link
Contributor Author

All CI tests are passing now. Good to go for the review @jtpio

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "@jupyterlab/github",
"version": "3.0.1",
"version": "4.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Should we revert the version back to 3.0.1 here for now?

When we will be making the release, we can specify 4.0.0 to the releaser. Otherwise if the version is already 4.0.0, the releaser will not allow setting the same version (4.0.0).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True that!

jupyter-releaser will take care of version bumping
Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!!

@jtpio jtpio merged commit b711c4a into jupyterlab:master Aug 2, 2023
9 checks passed
@welcome
Copy link

welcome bot commented Aug 2, 2023

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@jtpio
Copy link
Member

jtpio commented Aug 2, 2023

Waiting for #147 to be able to do the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

migrate github extension to run with jupyterlab 4
2 participants