-
Notifications
You must be signed in to change notification settings - Fork 308
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
pyproject.toml: clarify build system version #634
pyproject.toml: clarify build system version #634
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
Codecov Report
@@ Coverage Diff @@
## master #634 +/- ##
==========================================
- Coverage 77.77% 77.73% -0.04%
==========================================
Files 110 110
Lines 10444 10444
Branches 1415 1415
==========================================
- Hits 8123 8119 -4
- Misses 1921 1924 +3
- Partials 400 401 +1
Continue to review full report at Codecov.
|
It wasn't immediately clear to me if ~=0.9 translated to (A) |
@@ -1,5 +1,5 @@ | |||
[build-system] | |||
requires = ["jupyter_packaging~=0.9,<2"] | |||
requires = ["jupyter_packaging~=0.9"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want this to be >=0.10,<2
since we are compatible with the planned 1.0 interface for Jupyter-packaging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that if you want, just didn't want to change the current behaviour without a jupyter expert to tell me to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that mean this is no longer compatible with 0.9?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, this just sets a minimum on what will be used in the build environment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm comfortable with changing the upper bound since 1.X hasn't been released yet anyway, but I'm not sure if I'm comfortable with changing the lower bound. If this currently builds with 0.9, I see no reason to increase the lower bound.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. 😄
@vidartf is this ready to merge or would you like to see any of the additional changes recommended by @blink1073? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM.
See jupyter/jupyter-packaging#120