-
Notifications
You must be signed in to change notification settings - Fork 0
Gf impl code review suggestions #1
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
Gf impl code review suggestions #1
Conversation
…rings, with deprecation.
|
So far tests are passing locally. @adnanbhat7 I think you can approve the CI run here to make sure everything is working (although we'll still need to do that on the main repo as only some tests will work on your repo). |
|
@vincentmacri Thanks for the review and for raising the PR! I've approved the CI . |
|
I should not merge it until the failing checks are resolved. The documentation build failure is irrelevant, there's an existing issue with building the docs on CI. Basically we are hitting disk usage limits on the doc build. The build and test failure was spurious, I couldn't reproduce it locally. So I think this is ready to merge into your branch.
I made this PR against your |
e98ec9c
into
adnanbhat7:adnanbhat7/fixes-GF_imple
Cherry-pick sagemath/pull/35609 and resolve all the conflicts. Also some minor suggestion.
This looks like a lot of changes but it's almost entirely changing
impltoimplementationin various places.I'm running tests locally, and will unmark this as a draft once tests are passing.