use graphql's built-in version object by bollwyvl · Pull Request #3622 · strawberry-graphql/strawberry

bollwyvl · 2024-09-11T12:25:58Z

Description

Alternative to to #3621.

Uses graphql.version.VersionInfo instead of packaging to do version sniffing instead of packaging (or even importlib.metadata.version, as that only provides strings).

That API has been there for five years, certainly above the current minimum dependency.

Types of Changes

Issues Fixed or Closed by This PR

n/a

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Use GraphQL's built-in VersionInfo for version checking instead of the packaging library, simplifying dependencies and ensuring compatibility with GraphQL core.

Bug Fixes:

Replace the use of the packaging library with graphql.version.VersionInfo for version checking, ensuring compatibility with GraphQL core's built-in version object.

Chores:

Add a RELEASE.md file to document the release type as a patch and note the update in version checking.

for more information, see https://pre-commit.ci

codecov · 2024-09-11T12:32:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.75%. Comparing base (4ada2cc) to head (30e90fb).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3622      +/-   ##
==========================================
- Coverage   96.77%   96.75%   -0.02%     
==========================================
  Files         521      521              
  Lines       33731    33715      -16     
  Branches     5627     5622       -5     
==========================================
- Hits        32643    32622      -21     
- Misses        856      861       +5     
  Partials      232      232

patrick91 · 2024-09-11T12:36:16Z

-from packaging.version import Version
+from graphql.version import VersionInfo, version_info

-IS_GQL_33 = Version(graphql.__version__) >= Version("3.3.0a")
+IS_GQL_33 = version_info >= VersionInfo.from_str("3.3.0a0")


Ah nice! I think I like this more 😊

assuming graphql core will keep exporting VersionInfo :D

Sure, always a concern. But given the tightness of the pin, it seems unlikely this project would be caught unaware.

yup, I'd say let's merge this :)

can you add a RELEASE.md file?

release type: patch This release updates how we check for GraphQL core's version to remove a dependency on the `packaging` package

something like this 😊

codspeed-hq · 2024-09-11T12:37:27Z

CodSpeed Performance Report

Merging #3622 will not alter performance

_{Comparing bollwyvl:gh-3621-use-graphql-versioning (30e90fb) with main (a1f3275)}

Summary

✅ 15 untouched benchmarks

sourcery-ai · 2024-09-11T19:24:31Z

Reviewer's Guide by Sourcery

This pull request updates the version checking mechanism for GraphQL in the Strawberry project. It replaces the use of the packaging library with GraphQL's built-in VersionInfo object for version comparison. This change simplifies the dependency structure and potentially improves performance.

File-Level Changes

Change	Details	Files
Replace packaging library with GraphQL's built-in VersionInfo for version checking	Import VersionInfo and version_info from graphql.version instead of using packaging.version Replace Version(graphql.version) with version_info Use VersionInfo.from_str() to create version object for comparison Remove import of graphql module	`strawberry/utils/__init__.py`
Add release notes for the version checking update	Specify release type as patch Describe the change in version checking mechanism Mention the removal of dependency on the packaging package	`RELEASE.md`

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

patrick91

Thank you so much!

sourcery-ai

Hey @bollwyvl - 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: 2 issues found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

botberry · 2024-09-11T19:27:19Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

This release updates how we check for GraphQL core's version to remove a
dependency on the packaging package.

Here's the tweet text:

🆕 Release (next) is out! Thanks to Nicholas Bollweg for the PR 👏

This release updates how we check for GraphQL core's version to remove a
dependency on the `packaging` package.

Get it here 👉 https://strawberry.rocks/release/(next)

botberry · 2024-09-11T19:38:54Z

Thanks for contributing to Strawberry! 🎉 You've been invited to join
the Strawberry GraphQL organisation 😊

You can also request a free sticker by filling this form: https://forms.gle/dmnfQUPoY5gZbVT67

And don't forget to join our discord server: https://strawberry.rocks/discord 🔥

bollwyvl and others added 2 commits September 11, 2024 07:21

use graphql's built-in version object

171fd1f

[pre-commit.ci] auto fixes from pre-commit.com hooks

15f81c4

for more information, see https://pre-commit.ci

bollwyvl mentioned this pull request Sep 11, 2024

add packaging as a dependency #3621

Closed

11 tasks

patrick91 reviewed Sep 11, 2024

View reviewed changes

bollwyvl and others added 2 commits September 11, 2024 08:04

add RELEASE.md

8851dad

Add tweet file

30e90fb

patrick91 marked this pull request as ready for review September 11, 2024 19:24

patrick91 approved these changes Sep 11, 2024

View reviewed changes

sourcery-ai Bot reviewed Sep 11, 2024

View reviewed changes

Comment thread RELEASE.md Outdated

Comment thread RELEASE.md Outdated

botberry added bot:has-release-file labels Sep 11, 2024

patrick91 merged commit 22bd2fc into strawberry-graphql:main Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

use graphql's built-in version object#3622

use graphql's built-in version object#3622
patrick91 merged 4 commits intostrawberry-graphql:mainfrom
bollwyvl:gh-3621-use-graphql-versioning

bollwyvl commented Sep 11, 2024 •

edited by sourcery-ai Bot

Loading

Uh oh!

codecov Bot commented Sep 11, 2024 •

edited

Loading

Uh oh!

patrick91 Sep 11, 2024

Uh oh!

bollwyvl Sep 11, 2024

Uh oh!

patrick91 Sep 11, 2024

Uh oh!

codspeed-hq Bot commented Sep 11, 2024 •

edited

Loading

Uh oh!

sourcery-ai Bot commented Sep 11, 2024 •

edited

Loading

Uh oh!

patrick91 left a comment

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

botberry commented Sep 11, 2024 •

edited

Loading

Uh oh!

botberry commented Sep 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

bollwyvl commented Sep 11, 2024 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

Summary by Sourcery

Uh oh!

codecov Bot commented Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

patrick91 Sep 11, 2024

Choose a reason for hiding this comment

Uh oh!

bollwyvl Sep 11, 2024

Choose a reason for hiding this comment

Uh oh!

patrick91 Sep 11, 2024

Choose a reason for hiding this comment

Uh oh!

codspeed-hq Bot commented Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #3622 will not alter performance

Summary

Uh oh!

sourcery-ai Bot commented Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide by Sourcery

File-Level Changes

Uh oh!

patrick91 left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

botberry commented Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

botberry commented Sep 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bollwyvl commented Sep 11, 2024 •

edited by sourcery-ai Bot

Loading

codecov Bot commented Sep 11, 2024 •

edited

Loading

codspeed-hq Bot commented Sep 11, 2024 •

edited

Loading

sourcery-ai Bot commented Sep 11, 2024 •

edited

Loading

botberry commented Sep 11, 2024 •

edited

Loading