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

Pin setuptools-scm dependency version #2450

Closed
wants to merge 2 commits into from

Conversation

FabianNiehaus
Copy link
Contributor

@FabianNiehaus FabianNiehaus commented Aug 27, 2021

Description

The setuptools-scm dependency in setup.cfg did not have a version specified, leading to the issues described in #2449 after a faulty release of setuptools-scm was published.
To avoid this issue in the future, the last version before that faulty update is now pinned.

Checklist - did you ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary? -> Not needed
  • Add new / update outdated documentation? -> Not needed

@FabianNiehaus FabianNiehaus changed the title Pin setuptools-scm dependency version (#2449) Pin setuptools-scm dependency version Aug 27, 2021
@ichard26 ichard26 self-assigned this Aug 27, 2021
Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

I'm not a big fan of pinning setuptools-scm but if we are going to go down this route, I'd ask that we move the setuptools-scm requirement definition to the build-system.requires field in pyproject.toml.

@@ -13,6 +13,7 @@
trailing comma (#2384)
- Parsing support has been added for unparenthesized walruses in set literals, set
comprehensions, and indices (#2447).
- Pin `setuptools-scm` dependency version (#2449)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Pin `setuptools-scm` dependency version (#2449)
- Pin `setuptools-scm` dependency version (#2450)

We do it by PR number not issue number ^^

@FabianNiehaus
Copy link
Contributor Author

FabianNiehaus commented Aug 27, 2021

I'm not a big fan of pinning setuptools-scm but if we are going to go down this route, I'd ask that we move the setuptools-scm requirement definition to the build-system.requires field in pyproject.toml.

I am not too familiar with this, but setuptools-scm is already present in pyproject.toml (without version pinning). What is the purpose of setup.cfg in this case? Can we pin the version in pyproject.toml and remove setup.cfg?

@ichard26
Copy link
Collaborator

I honestly don't know why we specify setuptools-scm in both place, but given pyproject.toml is the newer standard (also isn't setuptools specific) it's probably best to pin there (although setup.cfg is by no means deprecated or anything like that). FYI looks like you got some trailing whitespace in setup.cfg. A comment explaining why we're pinning would be fantastic too but totally not neccesary.

@ichard26
Copy link
Collaborator

Sorry @FabianNiehaus but I ended up incorporating your PR into a new one with some tweaks applied on top since I need to get a release out today. I hope the credit via the co-author line is good enough! Thanks for your contributing! Feedback welcome to #2238 :3

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.

None yet

3 participants