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

PEP 621: Migrate from setup.cfg to pyproject.toml #11726

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Oct 30, 2024

Blocked by

Migrate settings from setup.cfg into pyproject.toml using ini2toml to do the file conversion and running pyproject-fmt and then validate-pyproject to validate the results.

@cclauss cclauss requested a review from a team as a code owner October 30, 2024 09:41
@cclauss cclauss requested a review from humitos October 30, 2024 09:41
@cclauss cclauss changed the title Setup.cfg to pyproject.toml PEP 621: Migrate from setup.cfg to pyproject.toml Oct 30, 2024
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Thanks for this @cclauss! I would feel good about updating our pattern here. The packaging configuration is not something we make use of really, so this is pretty low stakes. This is not a PyPI package and the only place we use this is during release operations.

I think if we update this to work with bumpver it should be okay, I might also be missing something. @humitos am I missing anything?

pyproject.toml Outdated
push = "False"

[tool.bumpver.file_patterns]
"setup.cfg" = '''
Copy link
Contributor

Choose a reason for hiding this comment

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

We use bumpver during releases, this needs updated and tested for this change to work for us. The filename and patterns might need updated.

pyproject.toml Outdated
Comment on lines 41 to 43
current_version = '"11.11.0"'
version_pattern = '"MAJOR.MINOR.PATCH[TAGNUM]"'
commit_message = '"Bump version {old_version} -> {new_version}"'
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the double quotes necessary here? This looks wrong to me, but it would be worth testing bumpver.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I follow. That seems like another similar change, just on the bumpver package. I meant we need bumpver to work for our repository. We should be able to run the standard bumpver commands and all of the version strings noted in the configuration should update.

@humitos
Copy link
Member

humitos commented Oct 31, 2024

@humitos am I missing anything?

I'm not sure. I think we don't really use setup.cfg as you mentioned, because this is not a package. I understand the only relevant part of this is the bumpver section -- which it could be on its own configuration file probably and we can remove the setup.cfg completely. Maybe that make more sense that keep these other files updated?

In general, this work seems to be pretty low priority since we don't really use anything of this 😄

@agjohnson
Copy link
Contributor

I don't think we should break out bumpver etc configuration into individual files, I like making everything packages even if we are immediately using them this way.

But if we're upgrading to a new pattern here, we do need bumpver to work.

@cclauss
Copy link
Contributor Author

cclauss commented Nov 1, 2024

My sense is that this pull request is blocked by mbarkhau/bumpver#229 because it does not make sense for readthedocs.org to migrate to pyproject.toml until bumpver documents how to store its metadata in .toml files.

I have always used bump-my-version so I have not had this issue before.

@agjohnson
Copy link
Contributor

Bumpver already support this:

https://github.com/mbarkhau/bumpver?tab=readme-ov-file#configuration-setup

That PR does just look like bumpver changing to pyproject.toml for it's own package, not actually supporting any further parsing of pyproject.toml as a library.

The format of the configuration around patterns does indeed seem incorrect in this PR, given the example pyproject.toml.

@cclauss
Copy link
Contributor Author

cclauss commented Nov 1, 2024

Thanks for the suggestions. Configuration fixed.

% bumpver update --dry --patch

INFO    - fetching tags from remote (to turn off use: -n / --no-fetch)
INFO    - Latest version from VCS tag: 2.1.3
INFO    - Working dir version        : 11.11.0
INFO    - Old Version: 11.11.0
INFO    - New Version: 11.11.1

--- docs/conf.py
+++ docs/conf.py
@@ -81,7 +81,7 @@
 
 master_doc = "index"
 copyright = "Read the Docs, Inc & contributors"
-version = "11.11.0"
+version = "11.11.1"
 release = version
 exclude_patterns = ["_build", "shared", "_includes"]
 default_role = "obj"
--- package.json
+++ package.json
@@ -1,6 +1,6 @@
 {
   "name": "readthedocs",
-  "version": "11.11.0",
+  "version": "11.11.1",
   "description": "Read the Docs build dependencies",
   "author": "Read the Docs, Inc <[email protected]>",
   "scripts": {
--- pyproject.toml
+++ pyproject.toml
@@ -38,7 +38,7 @@
 github_repo = "readthedocs.org"
 
 [tool.bumpver]
-current_version = "11.11.0"
+current_version = "11.11.1"
 version_pattern = "MAJOR.MINOR.PATCH[TAGNUM]"
 commit_message = "Bump version {old_version} -> {new_version}"
 commit = "False"
--- readthedocs/__init__.py
+++ readthedocs/__init__.py
@@ -1,5 +1,5 @@
 """Read the Docs."""
 
 
-__version__ = "11.11.0"
+__version__ = "11.11.1"

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Nice! That looks better, but I did notice one missing pattern.

I'll let @humitos call it on merging this, as I think if this disrupts anything it will be the release process.

setup.cfg Show resolved Hide resolved
@humitos
Copy link
Member

humitos commented Nov 11, 2024

I don't think we should break out bumpver etc configuration into individual files, I like making everything packages even if we are immediately using them this way.
But if we're upgrading to a new pattern here, we do need bumpver to work.

That's my point about breaking it into its own file; avoid upgrading something we don't use to a new pattern.

If we don't want to put bumpver into a different file, I'd delete everything that's not used from pyproject.toml and keep only the bumpver configuration there.

However, as I said, this is low priority to me and there aren't too much incentives to upgrade to a new pattern if we are not getting any value from it --mainly because we don't use it.

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