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

CI fix for "Testing nbclassic" on macos / pypy #106

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

ericsnekbytes
Copy link
Collaborator

@ericsnekbytes ericsnekbytes commented May 31, 2022

This PR proposes bumping the version of pypy being tested to 3.8 (which allows our CI tests to pass), then possibly opening a new issue for revisiting the pypy testing strategy in the future. We should think about the purpose of this test:

  • Users of pypy3.7 on macos will have a hard time using notebook at all, as cryptography (a dependency) does not provide a wheel for pypy3.7 on macos
  • Wheels are already available for needed pypy3.8 dependencies so it's a simple one line change (our CI test passes with pypy3.8)
  • If we bump to pypy3.8, we do not need to maintain a build toolchain (on the CI system/runners) for the cryptography library / dependencies, which involves configuring a rust build toolchain, package manager and openssl
  • We don't want to debug a from-source build of cryptography with this test, we just want a reasonable expectation that notebook works with pypy, so this keeps the test focused on the right things

@ericsnekbytes ericsnekbytes changed the title WIP CI edit, bump to pypy-3.8. CI fix for "Testing nbclassic" on macos / pypy Jun 1, 2022
@ericsnekbytes
Copy link
Collaborator Author

@Zsailer @blink1073 @jtpio @echarles @RRosio Does anyone have any comments on the proposal above?

@blink1073
Copy link
Contributor

Makes sense to me

@echarles
Copy link
Member

echarles commented Jun 4, 2022

Sounds good to me. @ericsnekbytes Could you please undraft this PR. Lets wait a few more days to gather any more feedback before merging.

@ericsnekbytes ericsnekbytes marked this pull request as ready for review June 6, 2022 11:23
@ericsnekbytes
Copy link
Collaborator Author

Sounds good to me. @ericsnekbytes Could you please undraft this PR. Lets wait a few more days to gather any more feedback before merging.

Just un-drafted the PR.

Copy link
Member

@echarles echarles left a comment

Choose a reason for hiding this comment

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

LGTM Thx @ericsnekbytes

@echarles echarles merged commit 462e202 into jupyter:main Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants