Re: [DISCUSS] Git commits and metadata

Thanks David. On Thu, Mar 27, 2025 at 9:50 AM David Arthur wrote: > Commented-by: left any comment on the PR (any contributor) > Reviewed-by: did a full review on the PR (any contributor) > Approved-by: committer(s) who approved the PR This matches my understanding and captures contributors' int

Re: [DISCUSS] Git commits and metadata

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 We will log the jira in the commit title, so maybe we don't need to lo

Re: [DISCUSS] Git commits and metadata

I see two problems that this solves. 1) Gets the history of approver out of GitHub and into our log (no need to look back at the PR) 2) Gives proper attribution for the single person responsible for committing/approving the code (untangle all the "Reviewers:") And yea, I think it can be totally a

Re: [DISCUSS] Git commits and metadata

What's the burden if it's automated? I agree we should keep it simple for anything that has to be done manually. Ismael On Tue, Apr 1, 2025 at 5:45 PM Matthias J. Sax wrote: > Hi, > > I understand the problem of a long "Reviewers" line and line break, so > breaking it down into multiple lines,

Re: [DISCUSS] Git commits and metadata

Hi, I understand the problem of a long "Reviewers" line and line break, so breaking it down into multiple lines, one per reviewer make sense to me. I don't see much value in using all these other trailers personally, and all the question about "when to add whom to which trailer" kinda backs

Re: [DISCUSS] Git commits and metadata

hi David > David Arthur 於 2025年3月27日 晚上9:50 寫道： > > Commented-by: left any comment on the PR (any contributor) > Reviewed-by: did a full review on the PR (any contributor) > Approved-by: committer(s) who approved the PR If a committer leaves the comment without approve, he/she should be include

Re: [DISCUSS] Git commits and metadata

Another point of clarification, I think we should reserve the Approved-by for the committer(s) who approved the PR. If a non-committer approves a PR, I think it should appear as Reviewed-by. -David On Thu, Mar 27, 2025 at 5:58 PM Ismael Juma wrote: > Hi Josep, > > To clarify, there is no subj

Re: [DISCUSS] Git commits and metadata

Hi Josep, To clarify, there is no subjectivity as far as I can tell. The approved-by/reviewed-by trailers would be used if you used the PR `approve` button. The former for committers and the latter for non committers. Anyone else who left comments would be in the commented-by trailers. The latter

Re: [DISCUSS] Git commits and metadata

Hi David, I find having the "commented" and "reviewed" distinctions a bit subjective. In my opinion, distinguishing between "approved-by" and "reviewed-by" is as far as I would go. Regarding the Jira trailer, I don't have any strong opinion, but it does help in the case of a PR working on different

Re: [DISCUSS] Git commits and metadata

Thanks for the feedback! A few common answers first: I think "Approved-by" should be the only required trailer. Since approving a PR implies a review, I think we can keep the mandatory trailers just to a single one. "Co-authored-by" is added automatically by GitHub if a PR has commits from anothe

Re: [DISCUSS] Git commits and metadata

Hi David, In general, I'm in favor of adding information where reasonably possible. How are these header values populated by the merging committer? Magic or manual? I agree with others that adding so many additional "*-by" headers could be confusing, leading to inconsistent usage. Is the equiva

Re: [DISCUSS] Git commits and metadata

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 "a

Re: [DISCUSS] Git commits and metadata

Hi David, I am in favor of having a separate field for the people who approve the PR vs the ones who leave some comments. I actually brought this up when I initially joined the community (probably around a decade ago), but it didn't resonate at the time. :) It also makes sense to use a consistent