prometheus/prometheus Changelog Management

[Callum Styan]

Hi all,

I'd like to start a discussion around changing how we manage the prometheus/prometheus changelog, specifically the fact that the changelog is generated manually by the release shepherd as part of the release process.

We can discuss options for what the new process would look like, such as requiring PR's to include changelog entries before merging or the next release shepherd periodically updating the changelog prior to the release, in more detail later. However I'd first like to get a sense of whether anyone else feels strongly about either changing or not changing this part of the release process.

Thanks,

Callum.

[Frederic Branczyk]

I recall Simon having a tool that would largely generate the changelog automatically, that worked pretty well last time I was release shepherd. Otherwise I'm also happy to discuss a process like in Kubernetes where the changelog item is written into the PR. On Thanos we have in the PR template that people have ensured that the changelog item was added respective to the change. Seems like there are options, I personally would favor something that would be done at contribution time, so not all the responsibility falls on the release shepherd as it does today, and more generally it seems like the person contributing the change probably is also a good candidate to describe it in the changelog.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAN2d5OTjOrCfpRF_NXGcQB5nOz%3DVPgnz3LdEk15ucV4PFz%2B4BQ%40mail.gmail.com.

[Brian Brazil]

On Fri, 14 Feb 2020 at 07:10, Frederic Branczyk <fbra...@gmail.com> wrote:

I recall Simon having a tool that would largely generate the changelog automatically, that worked pretty well last time I was release shepherd. Otherwise I'm also happy to discuss a process like in Kubernetes where the changelog item is written into the PR. On Thanos we have in the PR template that people have ensured that the changelog item was added respective to the change. Seems like there are options,

 

I personally would favor something that would be done at contribution time, so not all the responsibility falls on the release shepherd as it does today, and more generally it seems like the person contributing the change probably is also a good candidate to describe it in the changelog.

This is additional friction to contributions, we already have enough fun getting the DCO signed. It's also an additional burden on every single PR, we need to individually figure out if it's worth mentioned in the changelog (many PRs aren't) and then get it in the right category, with good wording, and handling the regular conflicts as everyone would be touching the same lines in the same file.

Even with all that the release shepard would still need to go through all the commits and double check that nothing was missed, plus fixing poor wording. I don't think saving 2-3 minutes off a release is worth all these downsides.

Brian

 

On Fri, 14 Feb 2020 at 08:05, Callum Styan <callu...@gmail.com> wrote:

Hi all,

I'd like to start a discussion around changing how we manage the prometheus/prometheus changelog, specifically the fact that the changelog is generated manually by the release shepherd as part of the release process.

We can discuss options for what the new process would look like, such as requiring PR's to include changelog entries before merging or the next release shepherd periodically updating the changelog prior to the release, in more detail later. However I'd first like to get a sense of whether anyone else feels strongly about either changing or not changing this part of the release process.

Thanks,

Callum.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAN2d5OTjOrCfpRF_NXGcQB5nOz%3DVPgnz3LdEk15ucV4PFz%2B4BQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAOs1UmyOfHbC75bdk55frFQt-KYgD6cg7vh%2BCPSmVmMnSV3sng%40mail.gmail.com.

--

Brian Brazil

www.robustperception.io

[Bartłomiej Płotka]

Hi,

> Even with all that the release shepard would still need to go through all the commits and double check that nothing was missed, plus fixing poor wording. I don't think saving 2-3 minutes off a release is worth all these downsides.

It is 2h do it properly for mere mortals Brian (: Seriously, have been there.

I think we already had some discussion with Goutham about it. In Thanos, we require CHANGELOG entry. Sure - we will miss some, but the end job for release Shephard is then 80% quicker and easier. I don't think that's a big problem for the contributor. Why do we need to suffer if in most cases contributor is even happy to contribute CHANGELOG entry? If the contributor says "no, I don't know how or I am lazy", sure, we will do it for them, but only then.

I would vote for a PR template that encourages changelog entry and asking contributors for optional changelog entry addition.

There was a solid discussion with Goutham on IRC about the separate issues with this approach: Merge conflicts in CHANGELOG.md. I think we should treat this as a separate discussion - we might have just a tool e.g google doc to track this.

Kind Regards,

Bartek

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLrFL_kN28EiagWYFbKMr5XWC%2Bk7h8n9D8VijvmOnX_5Tw%40mail.gmail.com.

[Matthias Rampke]

In the exporters that I maintain I specifically ask contributors not to fill in the changelog. I want to keep a somewhat editorial voice there. I often rephrase changes to highlight what the change means for users, and usually provide extra remarks like upgrade instructions or deprecation notices.

Having changelog entries added as part of PR commits also leads to endless merge conflicts.

I usually update the changelog right after merging. I would appreciate building this into the PR flow in a way where I can write the changelog entry without having to use a command line.

In Kubernetes, this seems to be done automatically by bots based on a section in the PR description. A big benefit of that is that as committers, we can edit it during review.

My ideal flow would be:

- the PR template has an empty entry for the changelog. a <!-- comment --> encourages contributors to fill it in but notes that the maintainers will take care of it

- it also has an optional entry for additional changelog remarks (we can leave this out if it's too much)

- as the maintainer, if I want to change or edit it, I edit the PR description

- if we don't want an entry for the PR, we delete it or leave it empty

- once I hit merge, an automatic mechanism adds both to the changelog (can CircleCI commit?)

- when creating the release, the shepherd only looks over the changelog, possibly adds or consolidates notes about an overarching theme (say, if multiple PRs together introduce a change)

This allows users to contribute the changelog entry, but we can edit it without the back-and-forth of changing commits. It splits the responsibility between the committer (to edit the changelog entry, if one is desired, for the concrete change), and the release shepherd (to make sure the changelog as a whole is good). The release shepherd would no longer need to look at every merge since the last release. Having a "field" in the description makes it easy for committers to edit, but keeps the distinction between "what does this PR do" and "what does this mean for users".

/MR

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLrFL_kN28EiagWYFbKMr5XWC%2Bk7h8n9D8VijvmOnX_5Tw%40mail.gmail.com.

[Bartłomiej Płotka]

I think I like this idea of reusing a commit message for this! We can definitely build some automation around this and it looks like such workflow would be a huge improvement! 

Thanks Matthias.

Kind Regards,

Bartek

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5V9jDRn021txQ5CB5Cd2KTOyph5TAbtxa9cTbUXncothQ%40mail.gmail.com.

[Matthias Rampke]

No – I mean to explicitly not use commit messages or anything that requires the contributor to change. I want to keep it in the PR description that is editable through the GitHub UI.

/MR

[Paul Traylor]

Typically for my projects, I try to put the changelog entry in the
merge commit. So Something like

[BUGFIX] Fixes the bug #123

and then when writing a release, I run something similar to `git log
--first-parent --pretty=oneline` to build my changelog

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAMssQwZvLHSsFLa__12%2BAVMKhBw9h8226Xmh928JrWeUf5h%2Bfg%40mail.gmail.com.



--
Paul Traylor
http://kungfudiscomonkey.net/

[Bartłomiej Płotka]

Sorry, I somehow was treating those the same.  PR description would do even better agree!

Kind Regards,

Bartek

[Julien Pivotto]

On 14 Feb 13:07, 'Matthias Rampke' via Prometheus Developers wrote:
> No – I mean to explicitly *not* use commit messages or anything that

> requires the contributor to change. I want to keep it in the PR

> *description* that is editable through the GitHub UI.
>
> /MR

Like a prombot command

/changelog [BUGFIX]: Prevent OOM's when remote_reading with GRPC

> >>>>> <https://groups.google.com/d/msgid/prometheus-developers/CAN2d5OTjOrCfpRF_NXGcQB5nOz%3DVPgnz3LdEk15ucV4PFz%2B4BQ%40mail.gmail.com?utm_medium=email&utm_source=footer>
> >>>>> .

> >>>>>
> >>>> --
> >>>> You received this message because you are subscribed to the Google
> >>>> Groups "Prometheus Developers" group.
> >>>> To unsubscribe from this group and stop receiving emails from it, send
> >>>> an email to prometheus-devel...@googlegroups.com.
> >>>> To view this discussion on the web visit

> >>>> https://groups.google.com/d/msgid/prometheus-developers/CAOs1UmyOfHbC75bdk55frFQt-KYgD6cg7vh%2BCPSmVmMnSV3sng%40mail.gmail.com
> >>>> <https://groups.google.com/d/msgid/prometheus-developers/CAOs1UmyOfHbC75bdk55frFQt-KYgD6cg7vh%2BCPSmVmMnSV3sng%40mail.gmail.com?utm_medium=email&utm_source=footer>
> >>>> .

> >>>>
> >>>
> >>>
> >>> --
> >>> Brian Brazil
> >>> www.robustperception.io
> >>>
> >>> --
> >>> You received this message because you are subscribed to the Google
> >>> Groups "Prometheus Developers" group.
> >>> To unsubscribe from this group and stop receiving emails from it, send
> >>> an email to prometheus-devel...@googlegroups.com.
> >>> To view this discussion on the web visit
> >>> https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLrFL_kN28EiagWYFbKMr5XWC%2Bk7h8n9D8VijvmOnX_5Tw%40mail.gmail.com

> >>> <https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLrFL_kN28EiagWYFbKMr5XWC%2Bk7h8n9D8VijvmOnX_5Tw%40mail.gmail.com?utm_medium=email&utm_source=footer>
> >>> .

> >>>
> >> --
> >> You received this message because you are subscribed to the Google Groups
> >> "Prometheus Developers" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an
> >> email to prometheus-devel...@googlegroups.com.
> >> To view this discussion on the web visit

> >> https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5V9jDRn021txQ5CB5Cd2KTOyph5TAbtxa9cTbUXncothQ%40mail.gmail.com
> >> <https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5V9jDRn021txQ5CB5Cd2KTOyph5TAbtxa9cTbUXncothQ%40mail.gmail.com?utm_medium=email&utm_source=footer>
> >> .

> >>
> >
>
> --
> You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5Uj_G5uH_qienVu%2B1nWpspn2EZJBK1%2BRWtiuUi6zcfS5g%40mail.gmail.com.

--
(o- Julien Pivotto
//\ Open-Source Consultant
V_/_ Inuits - https://www.inuits.eu

[Ben Kochie]

DCO is a strawman argument. We're always going to have issues with submission quality.

I've had very good luck asking for changelog entries on the node_exporter.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAHJKeLrFL_kN28EiagWYFbKMr5XWC%2Bk7h8n9D8VijvmOnX_5Tw%40mail.gmail.com.

[Matthias Rampke]

That's neat! However, it has the same limitation as my current workflow (adding it to the changelog after merge) for me – I need to be at a computer to do it.

It often happens that I make the final review on PRs during my commute on my phone, and then I have to defer merging them because I can't (easily) edit the changelog there. To make a custom commit I would also need a git CLI. The workflow that I propose would work well for me because I can edit the PR description quickly online, or it's likely already settled, and I can get PR merge and changelog entry done in one smooth motion. #mobilefirst

/MR

[Matthias Rampke]

The friction is real – DCO is not a submission quality issue but a roundtrip one. This would be even more difficult with wording.

 I agree that in many cases contributors can write the changelog entry; having the field in the PR template would encourage them to do so proactively.

/MR

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CABbyFmoOTL3BvNguLk%3D%3DpNJi52zP458DugtpOTz0HAdQanyXgQ%40mail.gmail.com.

[Brian Brazil]

On Fri, 14 Feb 2020 at 12:50, Bartłomiej Płotka <bwpl...@gmail.com> wrote:

Hi,

> Even with all that the release shepard would still need to go through all the commits and double check that nothing was missed, plus fixing poor wording. I don't think saving 2-3 minutes off a release is worth all these downsides.

It is 2h do it properly for mere mortals Brian (: Seriously, have been there.

I'm not sure what you're doing, but what I do is get github to give me a list of all commits, and then open a new tab for each of them with middle click - ignoring the ones I know aren't relevant (e.g. doc changes). I then go through the tabs, copying&pasting the one line description (which will include the issue number) for the ones that are relevant, and editing where the description isn't great. Finally I'll do a pass over everything to ensure that related changes are grouped together etc.

Even with all these changes, the only thing you'd save is the copying&pasting bit as the release shepard still needs to do all of the rest which is why I'm saying 2-3 minutes.

I'm not in favour of making code reviews harder for the sake of saving a lesser amount of time during releases.

Brian

--

Brian Brazil

www.robustperception.io

[Brian Brazil]

On Fri, 14 Feb 2020 at 13:15, Matthias Rampke <m...@soundcloud.com> wrote:

The friction is real – DCO is not a submission quality issue but a roundtrip one. This would be even more difficult with wording.

I'm with Matthias on this one. We've often have PRs which are ready to merge, but have been sitting there months waiting for a DCO.

Brian

--

Brian Brazil

www.robustperception.io

[Ben Kochie]

The way I've been doing it in the node_exporter is to ask for the changelog entry, but I don't block on it. If the code is ok, but they don't add a changelog entry, I'll merge and do it myself.

Usually what saves the round trip time is I include a copy-paste text of exactly what I want in the changelog, rather than leave it up to them to have to figure it out.

For example: https://github.com/prometheus/node_exporter/pull/1580#issuecomment-569954208

[Ben Kochie]

But we don't have to block on changelog entries like we do for DCO legal requirements.

For example, in GitLab codebases, we have a CI test that warns if a changelog entry is missing. Unless you tag the review with a label.

[Callum Styan]

Even with all that the release shepard would still need to go through all the commits and double check that nothing was missed, plus fixing poor wording. I don't think saving 2-3 minutes off a release is worth all these downsides.

I agree with Bartek's response here, it's not a 2-3 minute process at all. Not everyone is familiar with as many areas of the prometheus/prometheus code as you are, and we're not reviewing or merging most of the PR's in areas we don't know about. Another factor is the fact that commit messages and PR descriptions vary widely, and in some cases merged PR's had no descriptions and commit messages that did not really accurately reflect the user facing change of the PR.

Even with all these changes, the only thing you'd save is the copying&pasting bit as the release shepard still needs to do all of the rest which is why I'm saying 2-3 minutes.

The point is the messages you're copy/pasting are not necessarily accurate, and again it's not 2-3 minutes.

DCO is a strawman argument. We're always going to have issues with submission quality.

Yes. While I think the delaying of PR merges and having to rebase because of CHANGELOG.md are valid concerns, I don't think we should choose not to adopt some improved changelog management based on these reasons. As you say, we're always going to have issues with varying quality of drive by submissions. We can only do our best to encourage people submitting those PR's to be good citizens and help make our lives easier. 

Right now I'm liking Matthias' idea:

My ideal flow would be:

- the PR template has an empty entry for the changelog. a <!-- comment --> encourages contributors to fill it in but notes that the maintainers will take care of it
- it also has an optional entry for additional changelog remarks (we can leave this out if it's too much)
- as the maintainer, if I want to change or edit it, I edit the PR description
- if we don't want an entry for the PR, we delete it or leave it empty
- once I hit merge, an automatic mechanism adds both to the changelog (can CircleCI commit?)

- when creating the release, the shepherd only looks over the changelog, possibly adds or consolidates notes about an overarching theme (say, if multiple PRs together introduce a change)

I also like the idea of a prombot command like Julien brought up.

[Simon Pasquier]

Correct, the PR is in the promu repository (I've updated it just now
to address comments from Brian though it should have been done long
ago):
https://github.com/prometheus/promu/pull/170

Right now, it leverages the PR labels to classify the change (BUGFIX,
CHANGE, ...) and it uses the PR's title as the changelog entry. It
wouldn't be hard to mimic what Kubernetes is doing and lookup the
changelog entry in the PR description as Matthias suggested. If
nothing is found it can always fallback to the title.
I agree that asking every PR to include the changelog update might not
be convenient (both for the contributor and the maintainer).

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAOs1UmyOfHbC75bdk55frFQt-KYgD6cg7vh%2BCPSmVmMnSV3sng%40mail.gmail.com.

[Matthias Rampke]

How do I make it so there is no entry?

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAM6RFu79WLX7ZZezxMvQdinV5VMdP-0pfPSfTPs45g7RO-qGcA%40mail.gmail.com.

[Julien Pivotto]

On 14 Feb 23:30, 'Matthias Rampke' via Prometheus Developers wrote:
> How do I make it so there is no entry?

There is still work to be done after the changelog is generated.

> > https://groups.google.com/d/msgid/prometheus-developers/CAOs1UmyOfHbC75bdk55frFQt-KYgD6cg7vh%2BCPSmVmMnSV3sng%40mail.gmail.com
> > .

> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Prometheus Developers" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to prometheus-devel...@googlegroups.com.
> > To view this discussion on the web visit

> > https://groups.google.com/d/msgid/prometheus-developers/CAM6RFu79WLX7ZZezxMvQdinV5VMdP-0pfPSfTPs45g7RO-qGcA%40mail.gmail.com
> > .

> >
>
> --
> You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5XcbOkOrPVrQ-Mzd-FX%2B7W%2B6kdL_uKOCQ%3DLn1X8f2%2BdtQ%40mail.gmail.com.

[Bjoern Rabenstein]

Just my 2¢:

I'm firmly in the camp of hand-creating the CHANGELOG. I do so many
editorial work on it that a pre-populated CHANGELOG might easily
create more work than it saves. I do have to understand anyway what
each change is doing as we are supposed to filter out changes that are
not user-visible/-impacting (which could as well be that a
user-visible change was introduced with its corresponding CHANGELOG
entry, but later it was removed again before a release happened).

I'm essentially doing what Brian is doing when I cut a release. I only
found that problematic after releases were procrastinated for too long
and the CHANGELOG exploded. With the fixed cadence in
prometheus/prometheus, that isn't really a problem anymore (but 2.16.0
was a biggie nevertheless).

What is a problem is unclear or incomplete commit and/or PR
descriptions. I guess we can all agree to ask for those in reviews.

I don't feel strongly about how in detail we want to encourage good
commit and PR descriptions. If we put a hint in there to suggest a
changelog line if applicable, I'm all for it. If people want a
specific keyword to autoextract it, sure, as long as I don't have to
use it and it doesn't make it harder to contribute.

I would not like to directly edit the CHANGELOG.md file in each PR,
for all the reasons already stated.

--
Björn Rabenstein
[PGP-ID] 0x851C3DA17D748D03
[email] bjo...@rabenste.in

[Łukasz Mierzwa]

Adopting one of many variants of semantic commits and using one of many tools that generates a changelog from it can save some time there.

Examples: example: https://www.conventionalcommits.org/, https://keepachangelog.com/

Going that path is easier if one uses commit message linter, to ensure all comit messages follow required format - https://github.com/conventional-changelog/commitlint

That being said from a purely user oriented perspectice I always find it useful when changelog includes a section that gives me a human readable overview of what's included in given release, that is difficult to automate but can be added on top of automated changelog.

[Julius Volz]

I'm with Björn and Brian on keeping the CHANGELOG.md manually curated. You need to understand all relevant changes since the last release anyway as the release shepherd, and it's much easier and less overall overhead to weigh everything centrally in the end than trying to get everyone to formulate changelog entries for every PR that in the end result in a cohesive whole.

--

You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/da403d28-cfad-41c2-8d0b-91168be070f5%40googlegroups.com.

[Simon Pasquier]

On Fri, Feb 14, 2020 at 11:30 PM 'Matthias Rampke' via Prometheus
Developers <prometheus...@googlegroups.com> wrote:
>
> How do I make it so there is no entry?

If you mean "ignore a PR that shouldn't appear in the changelog" then
all PRs that aren't labelled will added as comments to the
CHANGELOG.md file.
From there you're still free to rearrange whatever has been produced.

Here is an example with Alertmanager:
https://github.com/simonpasquier/alertmanager/commit/7fde3cc25471c626e82def873a01a9924e1816ae

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAFU3N5XcbOkOrPVrQ-Mzd-FX%2B7W%2B6kdL_uKOCQ%3DLn1X8f2%2BdtQ%40mail.gmail.com.

[Callum Styan]

I'll leave it up to the next few release shepherds to decide if they want to revisit this topic. I'll do so myself next time I'm the release shepherd.

Thanks for the discussion everyone!