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

improve changelog/release handling #321

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Nov 17, 2024

move changelog&version check to pre-commit and to a separate CI job that skips on skip-release label

Remaining quirks:

  1. The job won't re-run automatically on label assignment. Claude is telling me I can get there by splitting up the workflows across files, I'll give that a try later.
  2. A CHANGELOG run will fail after getting merged to main, because it loses the label. Not sure how to resolve that neatly; maybe just always pass --allow-future if it triggers on push.

The test file is kind of ugly now in that it both looks like a pytest file and doesn't. I should probably add some comments at the very least.

EDIT: actually, I shouldn't skip the CI job with the skip-release label - I should just make that pass --allow-future as well.
(also the flag is somewhat opaquely named, probs time to do a proper argparse solution).

It's also not great that skip-release doesn't actually skip a release attempt, afair we just rely on pushing-the-same-version-again to twine to not do anything.

@jakkdl
Copy link
Member Author

jakkdl commented Nov 17, 2024

also it doesn't seem to work, at least not when re-running it after adding the label.

@Zac-HD
Copy link
Member

Zac-HD commented Nov 17, 2024

I think we've now hit my "not actually worth the trouble" threshold, let's stick with the status quo?

@jakkdl
Copy link
Member Author

jakkdl commented Nov 18, 2024

The status quo definitely needs changing. I just realized that I forgot to undo the temporary changelog edit in #316 when releasing #317 (and I realized that... as I forgot to update the changelog in #323 before pushing (although that was a different problem of not updating the changelog at all, lel)). But I could look at making a simpler solution

@jakkdl
Copy link
Member Author

jakkdl commented Nov 22, 2024

okay yeah there's a pretty simple way forward. Let's skip the label, and either have CI always allow FUTURE (but with a check so we don't release); or have CI always error on FUTURE (but it's tested in a separate job that's not required to merge, and main/ will error until we release). Let me know if you have any preferences between those two.

Will keep pre-commit, and fix update_version() that's been silently trying to edit README.md despite the example pre-commit config no longer being there.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines +73 to +74
- name: Test changelog & version python tests/test_changelog_and_version.py --ensure-tag
run: python3 tests/check_changelog_and_version.py
Copy link
Member

Choose a reason for hiding this comment

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

Name is inconsistent with command now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants