-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add support for author and maintainer fields #45
Conversation
I'll have to come back to this and figure out why it's breaking on 3.6 |
Ahh of course, I'm jumping the gun reviewing this with Python 3.6 being recalcitrant … but I can give it another quick look afterwards. |
Ahh… spotted what is wrong… change in behaviour between Python 3.10 and later. Dummy project for testing:
In Python 3.6:
Python 3.9:
Python 3.10:
You might need to check for the name/email being |
I went dependency-hunting and found that the change in behavior with respect to Python 3.6 only supports up to |
f80e21c
to
ab423c2
Compare
This implementation assumes that the author, author_email, maintainer, and maintainer_email strings are comma-separated lists of authors or maintainers respectively. A number of projects (including this one) use that method to list multiple authors/maintainers (as per e.g. https://stackoverflow.com/a/10005265), and for projects that list only a single person in the field, this won't hurt. We can expand this logic in the future to support other methods of specifying multiple authors/maintainers if necessary, perhaps even to the point of adding a command-line option to inform the plugin how to parse those fields.
These tests verify that WritePyproject._transform_contributors() does something reasonable in every case I could think of.
These tests run various combinations of author and maintainer metadata through the plugin. The tests only verify the "typical" cases; I'm leaving it to the unit tests for _transform_contributors() to ensure that weird cases are handled in the proper way. I made a judgment call to treat any missing metadata as an empty string, but to never generate an author or maintainer entry in pyproject.toml unless at least one of the name or email address is nonempty. This was an arbitrary choice, at least in the sense that I didn't do any research to determine how those cases are supposed to be handled (if there is any standard for them). If necessary we can fix up the behavior later. I wrote these tests to use the project fixture rather than manually constructing Distribution objects because if I try the latter way, it looks like setuptools_scm takes over and computes the version number from the actual setuptools-pyproject-migration project, rather than using the hard-coded version 0.0.1, which causes the tests to fail. Hopefully in a future commit we can fix that.
This fixes the test which was (intentionally) broken by adding support for the author and maintainer fields. I noticed that test_everything() actually did not include maintainer metadata before, so I added it - just a copy of the authors.
ab423c2
to
2c5cc38
Compare
I think I have this ready now. One note: I tried changing some of the tests to directly use |
Oh, and just something to be aware of: I expect that this is going to have a merge conflict with #46, which one of us is going to have to fix, but it shouldn't be hard to deal with. |
Whoops, I forgot to change the assignee when this was ready |
This pull request adds support for the author and maintainer fields in the plugin, and adds a bunch of tests to verify that it's working properly.
I made some arbitrary (but sensible) choices about how to handle unusual cases, like names without email addresses or vice versa, or extra commas - basically it just substitutes in an empty string any time something would be missing. I don't know if that aligns with how other tools work, but if we need to change that behavior later, it'll be easy to do so.
Closes #28