Skip to content

Adopt hatchling, plugins for python package build #371

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

Merged
merged 9 commits into from
Jan 12, 2023

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Jan 7, 2023

References

Elevator Pitch

  • adopt hatchling and some plugins:
    • hatch-jupyter-builder handles the jlpm junk
    • hatch-nodejs-version fetches metadata from package.json
  • normalize dependencies and metadata
    • docs/requirements.txt left to avoid node on RTD

  • add CI caching
  • quick pass over user/dev docs
  • normalize the etc folders with their destination structure
    • didn't do this for the nb/labextension due to path lengths (e.g. windows)

@bollwyvl bollwyvl changed the title Adopt hatchling+plugins for python package build Adopt hatchling, plugins for python package build Jan 7, 2023
@bollwyvl bollwyvl marked this pull request as ready for review January 7, 2023 23:56
"jupyter_server_proxy/labextension",
]
exclude = [
".git",
Copy link

Choose a reason for hiding this comment

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

hey! just FYI this is always excluded

Copy link
Member

@consideRatio consideRatio 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 soo much for your work on this @bollwyvl!!!

Overall this LGTM but there was some changes I wished for with associated code suggestions.

  1. Removal of set -eux
  2. Removal of cache for python packages (but not for installing yarn.lock)
  3. Revert addition of jupyter-server-proxy[docs] as a pip install target

consideRatio and others added 6 commits January 11, 2023 12:53
I think `-e` to error early and `-x` to print statements is the default in a github actions runner already. The addition of `-u` to error on using unset environment variables is the addtion I think isn't a default.

If we need a stricter shell script execution, we can use `defaults.run`, see https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#defaultsrun.
@bollwyvl bollwyvl requested a review from consideRatio January 11, 2023 13:50
@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Jan 11, 2023

In doing this, I'd recommend as well that we add a .binder/{environment.yml,postBuild} that exercises as much of this as possible, as well as the contrib stuff, but that can be another day.

Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

Besides the final comment, this LGTM!

Do you consider this ready for merge as well at this point @bollwyvl?

@bollwyvl
Copy link
Collaborator Author

Yes, I've no further changes planned once it passes CI.

Run mkdir jsdist
  mkdir jsdist
  cd labextension
  jlpm pack --filename ../jsdist/labextension-jlpmpack.tgz
  cd ../jsdist && sha256sum * | tee SHA256SUMS
  shell: /usr/bin/bash -e {0}
/home/runner/work/_temp/395963e9-5a5b-4889-943b-08c64b11e3e6.sh: line 3: jlpm: command not found

I'll change this to just use npm pack.

@bollwyvl
Copy link
Collaborator Author

Publish is passing, so this is probably good. Happy to make any other fixes, or let others do so... feel free to squash merge.

@consideRatio consideRatio merged commit 8a24161 into jupyterhub:main Jan 12, 2023
@consideRatio
Copy link
Member

Wiee nice thank you @bollwyvl!!! Super happy about this being modernized!!

I went for a normal merge as I think a few jupyterhub team members prefer we use that consistently.

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.

Modernize packaging infrastructure
3 participants