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

Replace use of scripts with entry_points #237

Merged
merged 1 commit into from Apr 14, 2022

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Apr 5, 2022

References

Changes

  • replaces use of scripts with entry_points: console_scripts
    • this generally generates better/more secure scripts that are more predictable for downstream packagers e.g. debian, conda-forge
    • also more cross-platform-compatible, which is a story for another day...

@welcome
Copy link

welcome bot commented Apr 5, 2022

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! 🎉

@bollwyvl bollwyvl changed the title Replace use of scripts with entry_point Replace use of scripts with entry_points Apr 5, 2022
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.

This looks good to me, but I'm not so confident on this technically etc so I'll ping someone else for review as well.

@minrk and @rkdarst I saw that this project isn't getting so much maintainer attention and hoped to ensure a few simpler PRs at least got some attention. As I'm not an active user myself so I only dare merge things that seems very clear to not break something, perhaps you could have a look at this small PR?

@bollwyvl bollwyvl closed this Apr 6, 2022
@bollwyvl bollwyvl reopened this Apr 6, 2022
@bollwyvl
Copy link
Contributor Author

bollwyvl commented Apr 6, 2022

Here's the corresponding PR on jupyterhub itself:

jupyterhub/jupyterhub#2313

@consideRatio
Copy link
Member

Nice that makes me quite confident this is good to go, I'll go for a merge if this grows stale.

Btw, test failures are unrelated and fixed by #241.

@consideRatio
Copy link
Member

consideRatio commented Apr 14, 2022

Given that this pattern was demonstrated sensible in jupyterhub without other changes needed and the PR seemed to go stale I'll go for a merge on this now that also all tests has passed by fixing it in other PRs.

@consideRatio consideRatio merged commit d4d6593 into jupyterhub:master Apr 14, 2022
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.

Replace scripts with console_scripts
2 participants