Repository navigation
feat: add a changelog package that reads the git log - #6
Merged
tonyandrewmeyer merged 31 commits intoSep 20, 2026
Merged
Conversation
tonyandrewmeyer
commented
Sep 14, 2026
tonyandrewmeyer
force-pushed
the
changelog-from-git-log
branch
from
September 14, 2026 23:15
57f5a11 to
62a18b7
Compare
tonyandrewmeyer
marked this pull request as ready for review
September 15, 2026 00:05
james-garner-canonical
approved these changes
Sep 17, 2026
james-garner-canonical
left a comment
james-garner-canonical
left a comment
Contributor
There was a problem hiding this comment.
One major concern: we should preserve unexpected commit types in the changelog so that a human can resolve them.
Maintainability concerns: let's do a pass cutting the comments way down before merging; and let's drop the library public API if this is just going to be a CLI tool. If we do want this used as a library, it seems like the API could be a lot smaller.
Various minor comments throughout.
Comment on lines
+50
to
+54
| Entries may be email addresses or GitHub handles, with or without a | ||
| leading `@`, in any mixture: a caller assembling the list from a team | ||
| page has both to hand and should not have to decide which kind each one | ||
| is. Comparison is case-insensitive, since neither an email address nor a | ||
| GitHub handle distinguishes case. |
Contributor
There was a problem hiding this comment.
Maybe we should just settle on one format for the team list, emails or Github users.
Collaborator
Author
There was a problem hiding this comment.
It's harder to match in that case. It's not about what the package gets passed, it's about what's available in the Git log.
Collaborator
Author
I'm ok changing to this, but for what it's worth, in canonical/operator#1855 this was brought up and went ahead with silent dropping. |
tonyandrewmeyer
commented
Sep 17, 2026
tonyandrewmeyer
commented
Sep 17, 2026
Collaborator
Author
Since we're changing this, I took the liberty of changing the other thing I think was a mistake: the release notes now have the |
Collaborator
Author
I'm not sure cutting the comments down makes it more maintainable, particularly in these agent-y days, which is why I'd left it somewhat wordy. But I'm trimmed it down a lot. The constants module is still on the long side, let me know if you would rather than was smaller too. |
tonyandrewmeyer
added a commit
that referenced
this pull request
Sep 17, 2026
…#7) `style/python.md` names seventeen ruff rule groups as the set the team standardised on, and the monorepo root turns on eight of them. Turning the rest on finds eight things in `ai-failure-notifier`, and those eight are the only reason the standard is not at the root already. None of them is a behaviour change. * Two docstring summaries put in the imperative mood (`D401`). * `fetch_issue_texts` builds its list with `extend` rather than a loop that appends (`PERF401`). * `resolve_origin` discards `find_run_markers`' third value on purpose - the passed-in issue wins, because a marker we failed to find does not make the issue the wrong one - so it is `_` rather than a name nothing reads (`RUF059`). * One nested `with` in a test becomes one `with` with two contexts (`SIM117`). * The two `subprocess` findings are marked rather than changed. `gh` really is called by name and resolved from the runner's PATH, and the argv really is a list this module builds with no shell anywhere, so `S603` and `S607` get a `noqa` and the reason beside it - the same treatment `S607` already had. The test conftest imports `subprocess` only to patch it out, which is what its `S404` is about. 68 tests pass. The rule set itself moves to the root in #6, which is red on lint until this lands. --------- Co-authored-by: James Garner <james.garner@canonical.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lifts `parse_release_notes`, `format_release_notes`, `format_changes` and the commit-type-to-category mapping out of `canonical/operator`'s `release.py`, which is being replaced by three workflows. The formatting is the part that isn't specific to one repository; the version arithmetic, the file rewriting and the GitHub calls stay behind in operator. The one behaviour change while lifting is that `format_changes` takes the date as an argument rather than calling `datetime.datetime.now()` itself, which is what made it impossible to assert on. The package does no I/O at all now, so a conftest fixture blocks the clock to keep it that way. The tests use two real operator releases as fixtures, reconstructed from the repository's own history: 3.8.2 for the ordinary case (eleven of its twenty-three pull requests are `chore`, and none of them reach the changelog) and a trimmed 3.8.0 for the one breaking change merged into a 3.x release. There's no console script, because what a command line should look like depends on the workflow that calls it, and that workflow hasn't been written yet. Refs canonical/operator#2224 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DC6P7uEu7Qw7kh2tDBuJyB
The two things the package was missing before a release workflow can call it.
`infer_bump_size` reads the size off the same parse the changelog comes from,
`next_version` applies it, and a `changelog` console script wraps both of those
and the two formatters for a workflow step that can't `import`.
The version arithmetic came here rather than staying in operator. That is the
split the README on this branch already describes ("the formatting and the
version arithmetic are centralised here") but PLAN.md doesn't, so it is worth
saying why: a bump size is not usable without the arithmetic that applies it,
and leaving `bump_minor_version` and `bump_patch_version` behind would have
every adopting repository rewrite the same four lines. What stays behind is
what actually varies between repositories - which version to count from,
whether a `.dev0` goes on afterwards, the ops-scenario +5 rule, and knowing
which branches a `feat` has no business appearing on at all. `next_version`
raises on anything that isn't a plain `X.Y.Z` rather than guess, which is how
it declines to have an opinion about the rest.
A `!` counts towards a minor bump. The plan says a `!` doesn't infer a major
one, which leaves open what it does infer, and `parse_release_notes` moves a
`feat!` out of `feat` and into `breaking` - so a rule that read `feat` alone
would call a release whose only feature is a `feat!` a patch, and ship a
breaking change in a patch release. That is a bigger bend of the rules than
the one we have actually agreed to. operator's 3.8.0 shipped a `refactor!` in
a minor release, which is the fixture this is tested against.
The console script is the package's I/O boundary and the only part of it that
reads the clock, for `--date`'s default. The conftest fixture that keeps the
library clock-free now covers every library module instead of `_format` alone,
and `_cli` is deliberately outside it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012fHEMV23WkCiU8YoLdF6iW
The package took GitHub's generated release-notes text, which meant it read conventional-commit types off pull request titles. A title is written once, when the PR is opened; the convention governs the commits, and the squashed subject is what lands on the branch. It also meant a POST to releases/generate-notes, so the package that was meant to replace release.py's dependency on GitHub had quietly inherited it. `parse_git_log` is the primary input now, and the console script defaults to it. `parse_release_notes` stays, for a caller that has a release body in hand already. Three things come with it: * The pull request number comes from the `(#N)` a squash merge appends to the subject, and a change carries the number rather than a URL. `format_release_notes` builds the link back up from the number and a caller-supplied repo, so the no-I/O rule holds. A commit with no `(#N)` carries `None` and renders with no reference at all, rather than the `(#?)` that used to hide it. * A contributor from outside the team maintaining the repository is credited in the bullet, which is what operator's CHANGES.md has always done by hand. The team is a parameter, since it drifts and differs per repository, and an empty team credits everyone - over-crediting is visible in the draft release, and crediting nobody is not. * A revert of something in the same range cancels with it and neither appears. A revert of something already released is called out under Reverted, and a revert of a released `feat` goes to Breaking Changes, since taking away behaviour people may be relying on is a breaking change whatever the revert commit's own type says. operator's 3.8.1..3.8.2 and 3.7.1..3.8.0 render byte-for-byte identically from either input, which the suite checks. 125 tests, up from 57. The new commit fixtures are real operator history; the revert cases beyond the one real revert (#2568, of a chore) are constructed, and say so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018YMpsxxo5JJQMa8FGdMGwf
`preview = true` in `pyproject.toml` means `ruff format` reaches into Markdown code blocks, and the README's example had three keyword arguments sharing a line. No change to what the example does.
Three things, all documentation. The "Why the commits, and not the release notes" section was deleted on this branch and my last commit put it back, because I wrote the file from a clone taken before that deletion and replaced it wholesale. Deleted again. Two comments described a state this package has never been in on `main`: the conftest docstring said `format_changes` "used to" read the clock, and a test comment cited "the `(#?)` an earlier version of this rendered". Both now state the rule and the reasoning without the back-story. The `release.py` references stay, because that is a real file in `canonical/operator` and it is the specification these fixtures are checked against. 125 tests pass, ruff check and format clean.
`next_version('3.8.1', MINOR)` does not say what the first argument is, and
the two plausible readings - the version this release follows, and the version
being cut - differ by exactly one release. The docstring has a paragraph
explaining which one it is, which is a sign the signature should say so
instead.
Both arguments are keyword-only now: `next_version(previous=..., size=...)`.
Every caller in the package, the console script and the two documented
examples are updated.
`style/python.md` in canonical/charm-tech names the rule set we standardised on, and the monorepo root turns on eight of the seventeen. The rest are here, in this package's own config, because turning them on at the root would newly fail `ai-failure-notifier`, which is already merged, on eight findings that are not this package's to fix. Moving them up a level is the right end state and wants its own change. Nineteen findings, all documentation: missing blank lines after the last section of a docstring, six summaries not in the imperative mood, a summary running onto a second line, an unsorted import block in the test conftest, and the console script's module docstring, which quotes a shell line continuation and so has to be raw. Two flat-is-better-than-nested cleanups the guide asks for by name: `_team` flattened `--team a,b --team c` with a doubly-nested comprehension, and `parse_git_log` filtered a generator expression inside a list comprehension. Both are loops now. Not changed, deliberately. Internal imports stay `from ._module import names` rather than the guide's module-prefixed form: `ai_failure_notifier` next door does the same, and making one of two packages diverge is worse than either convention. `MINOR` and `PATCH` are compared with `==` because `BumpSize` is a `Literal`, not an `enum.Enum`, so the identity rule does not apply. 125 tests pass, ruff check and format clean.
`parse_release_notes` was the original implementation and was demoted rather than removed when `parse_git_log` took over. Nothing called it. The flag that reached it existed because the function did, and the one caller this package is being written for runs in a checkout, so it has the log and no reason to fetch generated notes instead. Keeping it was not free. It was a second path through every formatter, an `--input` choice on four subcommands, its own fixtures, and a permanent behavioural divergence the suite had to pin: a revert cannot be resolved from notes at all, so for a range containing one the two paths did not merely differ in detail, they disagreed about what belongs in the changelog. Gone with it: the `--input` flag, `credit_for_handle`, `CHANGE_LINE_REGEX`, `PR_LINK_REGEX`, `NEW_CONTRIBUTORS_REGEX`, the notes fixtures, and the tests that existed to describe the difference between the two paths. `_categories` returns categories rather than a pair, since the compare link now only ever comes from `--compare-url`. The tests that used a notes fixture as a convenient way to build categories are rebased onto the commit fixtures for the same two releases, so what they were actually checking is unchanged. 90 tests pass, ruff check and format clean.
A stray second blank line, left where the two notes-format regexes were cut out: every other constant in the file is separated by one, and ruff does not mind because two blank lines between top-level statements is ordinary PEP 8. Two comments went stale in the same cut. There is one parser now, not two, and the compare line is not passed through from anything -- the caller supplies the link and this is only the prefix GitHub uses, so that notes rendered here read the same as notes rendered there.
It took `full_changelog`, a pre-formatted line it appended verbatim, so the caller had to know the shape of it. That made sense while `parse_release_notes` existed: the line came out of GitHub's notes and went straight back out, and the package really was passing something through. With that gone, it was the one piece of the output format the library did not own. Headings, bullets, ordering, credits and the breaking-change preamble are all its own; only the closing line was handed in ready-made, and the console script had to import `FULL_CHANGELOG_PREFIX` to build it. So it takes `compare_url` now and renders the line itself. The prefix goes back to being a private detail of the format, which is what `_constants` says it is, and the caller supplies the one thing only it can know: the tags at either end of the range.
`COMMIT_SUBJECT_REGEX` claimed no repository in the estate uses a scope. That is wrong: `canonical/pebble` carries one on 60 of its last 298 conventional subjects, from `chore(deps)` through `feat(cli)` and `fix(overlord)` to `fix(cmdstate,wsutil)`. The behaviour does not change. Dropping the scope is a decision about what a changelog entry should read like: a reader wants what changed, the package it changed in is in the diff, and a dependency-bump-heavy range would otherwise render sixty near-identical prefixes. The comment now says that, and says where the scope would have to be carried if a repository ever wanted it rendered. Two tests instead of one, since the shape is real rather than hypothetical: the second is `fix(cmdstate,wsutil)`, a real pebble subject and the one most likely to be read as two groups by a regex written for one.
Naming the three dependencies `release.py` happens to have says nothing about this package, and it is a claim about a file in another repository that nothing here checks. The reason for an empty `dependencies` stands on its own.
`style/python.md` names one rule set for the team's repositories, so it belongs where the line length and the quote style already are: at the root, applying to every tool in the monorepo. Having it in one package's config made the standard a property of whichever package happened to adopt it, which is the opposite of the point. This turns up eight findings in `ai-failure-notifier`, fixed separately. Lint here is red until that lands.
The flag went with `parse_release_notes`; this line survived because the sweep for it looked for the words the prose used elsewhere and not for the flag itself.
"the way `ai-failure-notifier` is run" reads fine while there are two packages and worse with every one added, and the line below it already shows the whole invocation, so the comparison was carrying nothing.
"before you decide it's wrong" argues with the reader before they have said anything. The two bullets make the case on their own.
Review feedback on the package, in one pass. A commit type that is neither a category nor `chore`, and a subject that is not conventional at all, used to be dropped without trace, which makes a typo in a commit type indistinguishable from a deliberate omission. Both now land under `Uncategorised`, keeping the type where there is one, so that whoever proofreads the draft release is the one who finds them. `chore` moves from "absent from `CATEGORIES`" to an explicit `IGNORED_TYPES`, which is a better place to say that dropping it is deliberate. Bump-size inference ignores the new category, so a surprise can never quietly move a version number. The public API goes with it. Nothing imports this package and nothing is planned to: `release.py` is being replaced by workflow steps rather than ported, so the console script is the only caller there will be, and fourteen re-exported names were a promise made to nobody. The tests import the private modules directly, as `conftest` already did. The rest is what the reviewer asked for around that: `--team` takes one spelling instead of two, `_categories` and `_team` are inlined into `main`, the usage example moves out of the `_cli` docstring and into `--help` where a caller can see it, `_authors`' module docstring folds into the function it was describing, and the no-clock fixture discovers library modules rather than listing them, so a module added later is covered by default. The fixtures are invented now, people and prose both, and the tests are pytest-style throughout. No real name, email address or commit subject is left in the package. What the fixtures still preserve is structure -- merge order, the eleven `chore` pull requests that must not reach the output, a subject with a second colon in it, one with an apostrophe, two that collide -- and the test module's docstring says so, because inventing a replacement that drops one of those would quietly stop a test testing anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Tony Meyer <tony.meyer@gmail.com>
A `CHANGES.md` entry now ends each bullet with the full `https://github.com/owner/name/pull/N`, and a release-notes bullet with `in #N`, which is the way round GitHub makes the better of. A release body is rendered by GitHub, which turns `#N` into a link carrying the pull request's title; a `CHANGES.md` is read in an editor, on PyPI and in the docs as well, and only GitHub would make a link of the short form there. `--repo` moves with the links: `changes-entry` now requires it, and `release-notes` no longer needs it to build anything, though it still takes it to tell a `Reverts other/repo#N` line apart from one naming this repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other repository that selects all of `D` says which convention its docstrings follow: operator, charmlibs, jubilant and ops-harness all set `convention = "google"`, and concierge-py spells out by hand the exact list that setting disables. This repository selected `D` and said nothing, which leaves the rules belonging to the other conventions switched on. Two consequences, both now gone. The `D203`/`D213` ignores existed only to break up pairs that contradict each other, which the convention resolves on its own. And `D413` wanted a blank line between a docstring's last section and its closing quotes, which is why eight docstrings here carried one; a reviewer asked what that line was for, and the answer was "nothing". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adopting the team's ruff rules at the root left this package linted against rules it had never seen, since it extends the root configuration. Ten findings, none of them behaviour: `fetch_issue_texts` builds its list in one expression rather than appending in a loop. `resolve_origin` unpacked an `origin_issue` it then never used -- the number the workflow passed in wins, deliberately -- so it is discarded by name now. A `# noqa: BLE001` named a rule the set does not select, so the directive was dead and its reason survives as a plain comment. Four tests nested `mock.patch.object` inside `assertRaises`, and now use one `with`. `S404` and `S603` join the root ignores instead: shelling out is what this package is for, and operator, ops-harness and jubilant all ignore the pair for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The module docstring said the same thing three ways: that this is the I/O boundary, that everything under it is text in and text out, and how the test suite enforces that. The rule survives in four lines; the description of the fixture does not, since the fixture is right there in `conftest` and says so itself. `_today` picks up the one sentence a reviewer had to ask for: why it is called where `--date` is used rather than passed as its `default=`. The parser is built for every subcommand, and `bump-size` has no business reading the clock to answer "minor or patch". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The invented maintainers were charm puns, which read as the same joke as the contributors from outside the team. Ubuntu codenames say "this one is staff" at a glance, and the alliteration was already the shape the names had: Focal Fossa, Precise Pangolin and Trusty Tahr commit here, with Hardy Heron, Warty Warthog and Maverick Meerkat on the team list. Only the `@canonical.com` humans move. Ducky Debugger, Sidecar Sid and Bundle Ojoy are the outside-the-team cases those fixtures exist to draw, and the two bots are not people. Every handle is unregistered on GitHub, as the old ones were -- four of the LTS names were not, which is why the two release-less members are not LTS. One real first name went with them: a comment cited an operator commit "by Ben", and it now describes the shape of the subject instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A reviewer asked for the small example plus no more than a sentence or so for each oddity that needs commenting on. The example landed earlier; the bullets underneath it were still running to three sentences, the longest at 77 words. Every bullet is one sentence now. What goes with the trimming is the reasoning behind four of them: why a `!` does not infer a major bump, why the revert pairing key is the `Reverts owner/repo#N` line rather than a SHA, and why an adopting repository supplies no part of the type map. The first two are still in `_constants.py`, next to the constants they explain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`"assert", # S101` is what operator's list does, and it reads without a lookup. The codes stay as trailing comments, because a misspelled rule name is only a warning: ruff carries on and the ignore quietly stops applying, so the code is what a reader checks the name against. Selecting by name needs preview mode, which this config already sets for the formatter's bracket hugging. `select` cannot follow suit: a whole linter has no name ruff accepts, only its prefix, which is why those stay `"F"` with the linter named in a comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A bare `https://github.com/canonical/operator/pull/2684` in parentheses is a link everywhere the file is rendered, but it is also 50 characters of noise on a line whose point is the change. `([#2684](...))` renders as the `#2684` the hand-written entries always used, and still resolves in an editor, on PyPI and in the docs, which is why the URL was spelled out in the first place. Release notes are untouched: GitHub links a bare `#2684` in a release body itself, so `in #2684` stays as it is. The example in `_bullet`'s docstring also swaps a real contributor's handle for `@ducky-debugger`, the invented one the rest of the fixtures use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rebase onto main brought in canonical#7's version of the ai-failure-notifier style work, which suppresses S404/S603/BLE001 inline. This branch moves that pair to the root ignores in pyproject.toml instead, so the inline suppressions are now unused and ruff fails on them. Removing them is what the branch's own commit intended; the two landed independently and only collide once both are present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tonyandrewmeyer
force-pushed
the
changelog-from-git-log
branch
from
September 20, 2026 23:27
5a7201c to
4dc0d1f
Compare
tonyandrewmeyer
added a commit
to tonyandrewmeyer/charm-tech-code
that referenced
this pull request
Sep 23, 2026
canonical#6 listed it as `undocumented-public-inits`, which ruff does not know, so every run warned about an unknown selector and `D107` stayed on despite the comment saying `__init__` does not need a docstring. The rule's name is `undocumented-public-init`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The canonical/operator
release.pyturns a release range intoCHANGES.mdentries and a release-notes body, and every repo that wants the same thing has to copy it. This is that logic as a package, so ops, charmlibs, jubilant and the rest can share one implementation of "our changelog format".git log, rather than a draft release generated by GitHub.feat, or of anything carrying a!, is routed to Breaking Changes.Tests are built on two real operator releases as fixtures.
3.8.1..3.8.2and a trimmed3.7.1..3.8.0produce byte-identicalCHANGES.mdentries and release-notes bodies from either input path, against what those releases actually shipped - with the one deliberate exception of the author credits, which is the whole point of the fourth bullet.