-
-
Notifications
You must be signed in to change notification settings - Fork 543
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
Clean up python 3.8 leftovers #3740
Conversation
Reviewer's Guide by SourceryThis PR upgrades the project to Python 3.9 and removes some type casts and version constraints that are no longer needed. Class diagram showing typing changes in Python 3.9 upgradeclassDiagram
class AST {
+Index
+value
}
class ExprSlice {
+value
}
AST -- ExprSlice
note for ExprSlice "No longer needs type casting in Python 3.9+"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @DoctorJohn - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a note to the changelog/documentation about this breaking change, explaining why Python 3.9+ is now required and providing upgrade guidance for users still on Python 3.8.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
b663b61
to
8f35ea7
Compare
Apollo Federation Subgraph Compatibility Results
Learn more: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3740 +/- ##
=======================================
Coverage 97.22% 97.22%
=======================================
Files 502 502
Lines 33428 33428
Branches 5484 5484
=======================================
Hits 32499 32499
Misses 721 721
Partials 208 208 |
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.
Hey @DoctorJohn - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Hi, thanks for contributing to Strawberry 🍓! We noticed that this PR is missing a So as soon as this PR is merged, a release will be made 🚀. Here's an example of Release type: patch
Description of the changes, ideally with some examples, if adding a new feature. Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :) Here's the tweet text:
|
CodSpeed Performance ReportMerging #3740 will not alter performanceComparing Summary
|
2c6944d
to
379623e
Compare
21c9c29
to
dd5abd3
Compare
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.
Python 3.9 upgrade branch I was working on
OMG hahaha, I didn't know you were working on that lol
Let's join forces next time!
Description
This PR is a micro follow-up to #3730 and contains essentially the diff of Thiago's PR and a Python 3.9 upgrade branch I was working on. The diff amounts to the update of one 3.8 reference in our docs and the removal of an now unnecessary version constrain.
Types of Changes