Relax jupyter_events dependency requirement #57
Conversation
|
Thanks for submitting your first pull request! You are awesome! 🤗 |
|
@kevin-bates - cc'ing |
Codecov ReportBase: 85.65% // Head: 85.65% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
=======================================
Coverage 85.65% 85.65%
=======================================
Files 5 5
Lines 523 523
Branches 68 68
=======================================
Hits 448 448
Misses 53 53
Partials 22 22
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@akchinSTC - do you mind rebasing the PR and backing out the home URL change since it was addressed via #55? |
- Update dead link in project URL Signed-off-by: Alan Chin <akchin@us.ibm.com>
akchinSTC
force-pushed
the
relax-events-dep
branch
from
ce33212 to
b7335b4
Compare
@kevin-bates - done, my fault for not noticing. |
There was a problem hiding this comment.
Thanks @akchinSTC - these changes look good.
I would like @dlqqq to confirm the relaxation of the jupyter_events version before merging and have requested his review.
pyproject.toml
Outdated
| dependencies = ["jupyter_server>=1.15, <3", "jupyter_events~=0.5.0"] | ||
| dependencies = [ | ||
| "jupyter_server>=1.15, <3", | ||
| "jupyter_events" |
There was a problem hiding this comment.
I think we can just change this to:
"jupyter_events>=0.5.0,<1"
I think we do have a minimum version requirement on jupyter_events==0.5.0, though I admit I cannot recall the exact reason why.
There was a problem hiding this comment.
I'm OK with dropping the <1 version qualifier too, I just figured I'd add that in case v1 had any major breaking changes. But that could be premature reasoning.
There was a problem hiding this comment.
Yeah, given "0.x" releases can break APIs at any time, I'm not sure what a cap is going to do in this case, other than ensuring a release is necessary to support jupyter_events==1.0. I don't think the floor is necessary at this point either, but having a floor is fine.
There was a problem hiding this comment.
I think it's more practically plausible a 1.0 release would have breaking changes than a 0.x release, but again, you're right in pointing out that it makes no technical difference. Feel free to ignore my suggestion, I don't feel strongly on this topic.
I don't like the fact that this change leaves us open to breaking on every minor release of jupyter_events, but given that incompatible versions freak pip out, this is almost a case of "existing behavior".
I don't think the floor is necessary at this point either, but having a floor is fine.
Better safe than sorry? 😁
|
BTW, I'll release 0.7.0 pending merge and green CI, just for our beloved Elyra users 🤗. I've already moved any remaining open issues in the milestone to 0.8.0. |
Signed-off-by: Alan Chin <akchin@us.ibm.com>
|
@kevin-bates @dlqqq - thanks for the approvals and comments. Changes made and pushed! |
|
Congrats on your first merged pull request in this project! 🎉 |
|
Thank you very much! @akchinSTC 🤗 |
|
@akchinSTC 0.7.0 is released! 🎉 |
|
@dlqqq - can confirm this resolves the original issue, thank you! |
Opening this PR in response to an error I get when installing the Elyra package and following error