-
Notifications
You must be signed in to change notification settings - Fork 22
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
ci: automate releases, add check for py3.7, and create pr checklist #201
Conversation
since they must be md if we want, we could keep them by using and installing mdinclude (https://sphinx-mdinclude.omnilib.dev/en/latest/example.html)
since latest may refer to code that's still in development
my strategy is to use whatever was the current version at the time the package was added as a dependency
This reverts commit 2b5295f.
since they must be md if we want, we could keep them by using and installing mdinclude (https://sphinx-mdinclude.omnilib.dev/en/latest/example.html)
since latest may refer to code that's still in development
my strategy is to use whatever was the current version at the time the package was added as a dependency
This reverts commit 2b5295f.
…lab/TRTools into build/automated-releases
Hey Arya, thanks for doing this! Some minorish points: I personally liked having a detailed change log, explaining everything that's happening. Having it just be one-liners that link to PRs means you have to go digging to figure out what actually changed. Is there a way to copy the bodies of the PRs into the changelog in addition to the one line summaries? Regardless of the above, having PRs be self documenting for the change log is a big change for my own PR style and presumably a change for many other lab members. Normally I write them either sloppily, not thinking they'll be looked at much, or I write them in a detailed way for the reviewer. Now I'll need to write them for an unknown future audience with little presumed background so they can understand it as part of the changelog history. If we're making this change, we need these new requirements of PR etiquette to be very clearly stated somewhere in the TRTools documentation, and we should make a brief announcement to the lab that everyone will need to change their PR style. Wherever we document this, we also need to state that in a PR, if the if the scope of the PR changes, the original PR commit message will need to be edited to be kept up to date. People shouldn't have to read through the conversation history of a PR to understand what happened, and now they can't look to a manually written changelog for that purpose. Between steps 2 & 3 in PUBLISHING.rst there should be a description of what the person doing the review should be looking for. I would at least have them make sure that the changes have been documented appropriately in that PR. I'm open to having other review tasks their as well if you can think of some. Instead of deleting the release notes page, can we just have it consist of a single line link to the CHANGELOG on the github repo? |
Thanks for writing out all of your thoughts, @LiterallyUniqueLogin ! I agree that one liners aren't ideal because they require extra digging. I don't know if there's a way to include the entire PR body in the release notes, but one thing that I forgot to mention is that you can edit the contents of the text body of the release PR before it gets merged. And whatever you include there will get included in the CHANGELOG. I haven't used this (undocumented?) feature yet myself, but you can see an example of it in the release PR and release notes for Snakemake 8.0, which also uses the release-please GitHub action. So we can still manually write the CHANGELOG, if we'd like. Regarding your broader point about PR etiquette: I'm glad you also brought up guidelines for reviewers because I often struggle to identify the best way to review a PR myself. I'll try to add a section to the TRTools docs with a list of suggestions for PR reviewers, but I would feel better if there was also some input about all of this from others in the lab, perhaps on our Slack? I also think it would be nice to have some documentation, perhaps on the lab wiki?, that lists some general suggestions for pull request creators and editors, since a lot of this will also apply to haptools and other projects in our lab. What do you think? |
All good points! You really know your way around github best practices, that's a very useful skill. How about this then, Three more changes for this PR:
and then this PR should be good to go. Outside of this PR, we can chat with Melissa and decide if we want lab-wide PR creation/review guidelines, and if so, do we want to write them up for the lab wiki and/or solicit feedback. |
done! thank you for all of the comments and suggestions, @LiterallyUniqueLogin ! And regarding lab-wide PR creation/review guidelines: your plan sounds good to me |
This PR lets us automate the process of releasing new versions using Google's Release Please Github action. It works by creating (and maintaining) a "release PR" which, when merged, will update the changelog, bump the version number, and upload our source distribution and wheel to PyPI.
how is the version number determined?
The version number is determined automatically according to 1) semantic versioning and 2) the titles of each PR (which should follow the conventional commits spec). For example, if all of the recently merged PRs are prefixed with
fix:
, then the system will infer that all recent changes have been bug fixes, so it will only bump the "patch" version. However, if even one of the recently merged PRs is prefixed withfeat:
, then the system will bump the "major" version because it will infer that a new feature has been added tomaster
.This makes our versioning more interpretable to our users while also redistributing the work of maintaining the version onto each developer of our toolkit.
how is the changelog maintained?
The changelog for each release is simply the contents of the text body of the release PR. These text entries will themselves come from the titles of each of the PRs that have been merged since the last release. For example, this is what the changelog entry would look like if the following two PRs were merged into master since the last release:
Haplotypes.__iter__
Here is also a more recent example.
some caveats
fix:
. Or, if it is a new feature, it will need to prefixed byfeat:
. I've added another Github action to prevent merging of PRs that don't have thisother, somewhat unrelated, things in this PR
pytest.ini
and.pylintrc
into thepyproject.toml