hi David +100 to this change.
> signed-off-by I guess this is used to log the guy who does commit the PR? > Helped-by Is it equal to Co-authored-by? > Fixes: KAFKA-12345 <https://issues.apache.org/jira/browse/KAFKA-12345> We will log the jira in the commit title, so maybe we don't need to log it in the body? Best, Chia-Ping José Armando García Sancio <jsan...@confluent.io.invalid> 於 2025年3月26日 週三 下午10:37寫道： > Hi David, > > I like the motivation for this change but I have some suggestions. > > 1. To me the only required fields should be "reviewed-by" and > "approved-by". I don't fully understand the rest of the fields. I > would like to limit the burden on the committer to merge a change. > 2. How about extending "approved-by" to also include non-committer? To > me the important thing is to distinguish contributors that > participated in the discussion versus contributors (committers and > non-committers) that actually agree with the change and think that > should be included in Apache Kafka. > > Thanks, > -Jose > > On Tue, Mar 25, 2025 at 8:06 PM David Arthur <mum...@gmail.com> wrote: > > > > Hello Kafka community! > > > > I wanted to start a discussion around our Git commits and the metadata we > > keep in them. > > > > We have historically used the "Reviewers" Git trailer [1] to indicate who > > had reviewed a commit. Originally, it seems we used this field to > indicate > > the committer who approved the change (per our By-Laws). But over time, > its > > usage has expanded to include anyone (committer or not) who left a > comment > > on the PR. > > > > I think acknowledging reviews is very important for our community, and I > > want to continue doing this. > > > > I also think it is important to record the committer who approved a given > > PR. > > > > Besides improving the quality of our Git log, I'm raising this issue > > because of a limitation/quirk I've discovered in GitHub. While > researching > > https://github.com/apache/kafka/pull/19242 I found that GitHub will > > automatically re-wrap the text of the PR body to fit 72 characters. This > > will blindly break long "Reviewers" lines (which we regularly exceed). > This > > will make it difficult to easily find reviewers of PRs using the Git log. > > > > I would like to propose that we start using the following Git trailers in > > our PRs: > > > > * Reviewed-by: anyone who left feedback on the PR > > - * Approved-by: committers who approved the PR > > - * Helped-by: shout-outs for inspiration or significant help > > - * Signed-off-by: commit signatory > > - * Fixes: KAFKA-12345 < > https://issues.apache.org/jira/browse/KAFKA-12345> > > - * References: KIP-123, KAFKA-23456, apache/flink#1234, etc > > > > Each "-by" trailer should include only a single individual using the > > standard "First Last <email>" format. Multiple instances of the same > > trailer are allowed. Before merging, we would use a GitHub Action to > verify > > that at least one "Approved-by" is present. > > > > This accomplishes a few things: > > > > * Works around the GitHub 72 character limit > > * Keeps a cleaner record of reviewers vs approvers > > * Allows us to move metadata out of the commit subject, if desired (e.g., > > KAFKA-12345) > > > > Structuring the metadata in this way allows us to use standard git > commands > > to parse the commits. This helps us move towards a fully automated > process > > where we populate these fields from the GitHub Pull Request data. > > > > Let me know what you think! > > > > David Arthur > > > > [1] https://git-scm.com/docs/git-interpret-trailers > > > > -- > -José >
