Page MenuHomeFreeBSD

git hooks: Adjust hooks for the ports tree
ClosedPublic

Authored by 0mp on Apr 20 2021, 11:42 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 9, 1:47 AM
Unknown Object (File)
Fri, Mar 8, 4:25 PM
Unknown Object (File)
Fri, Mar 8, 4:25 PM
Unknown Object (File)
Fri, Mar 8, 4:25 PM
Unknown Object (File)
Fri, Mar 8, 4:25 PM
Unknown Object (File)
Fri, Mar 8, 4:25 PM
Unknown Object (File)
Fri, Mar 8, 4:25 PM
Unknown Object (File)
Fri, Mar 8, 4:25 PM

Details

Summary
  • Capitalize the topic line: this way the example is consistent with the desired style.
  • Update the description of MFH.
  • Point the Pull Request field to the ports repo on GitHub.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 38714
Build 35603: arc lint + arc unit

Event Timeline

0mp requested review of this revision.Apr 20 2021, 11:42 AM
0mp created this revision.

Change the commit message

Whoops, I upated the wrong Phabricator link. Sorry about that.

Bring back the correct patch.

Two thoughts here:

  1. Should we restore "Submitted by:" to this list?
  2. Should we examine $(make -V PKGORIGIN) to prepopulate the subject (first) line?

Two thoughts here:

  1. Should we restore "Submitted by:" to this list?

IMO if "Submitted by" gets restored, it should be noted that it's expected to be more of an anomaly than the norm -- reserved for situations where the original work was submitted by someone else, but changed substantially enough that the author should be updated.

IMO if "Submitted by" gets restored, it should be noted that it's expected to be more of an anomaly than the norm -- reserved for situations where the original work was submitted by someone else, but changed substantially enough that the author should be updated.

Interesting. I always thought of that for "Reported by," and treated "Submitted by" as the norm.

IMO if "Submitted by" gets restored, it should be noted that it's expected to be more of an anomaly than the norm -- reserved for situations where the original work was submitted by someone else, but changed substantially enough that the author should be updated.

Interesting. I always thought of that for "Reported by," and treated "Submitted by" as the norm.

In a scenario where someone submitted a patch, "Reported by" would imply an even further alteration where you didn't really use their patch at all but took it as an alert that the problem existed. In a Git world, if you used their patch then the author would reflect them and the committer would reflect you, and if you made substantial changes to the patch then the author can reflect you and "Submitted by" would note the author of the patch you based it off of.

I may be misunderstanding the process you're describing, but it sounds to me like you're talking about a situation involving merging a pull request where the author is annotated. If the submission came from a BZ bug report, is the "Submitted by" field not still relevant?

My brain is definitely adapting slowly to the new git world order.

  1. Should we examine $(make -V PKGORIGIN) to prepopulate the subject (first) line?

This will not work. The editor is started from the root of the project directory. The following line is what I add to my hook at the moment:

$(git status --untracked-files=no --short | awk '{print $NF}' | cut -d / -f 1-2 | sort -u)

I am not sure if it's pretty enough to get included.

I may be misunderstanding the process you're describing, but it sounds to me like you're talking about a situation involving merging a pull request where the author is annotated. If the submission came from a BZ bug report, is the "Submitted by" field not still relevant?

My brain is definitely adapting slowly to the new git world order.

From what I understand, it is not relevant because you can use git commit --author "Ports Contributor <ports.contributor@example.org>" in order to show who's the author.

I may be misunderstanding the process you're describing, but it sounds to me like you're talking about a situation involving merging a pull request where the author is annotated. If the submission came from a BZ bug report, is the "Submitted by" field not still relevant?

My brain is definitely adapting slowly to the new git world order.

Ok, let's start over. :-) The key point that I think we're missing here is that Git has a separate record of who wrote a change, vs. who committed a change. Take a look, for example, at this in base: git show --pretty=full 600bd6ce0639c:

commit 600bd6ce0639c84b763516477250df5964e8edf6
Author: Kurosawa Takahiro <takahiro.kurosawa@gmail.com>
Commit: Kristof Provost <kp@FreeBSD.org>

    pfctl, libpfctl: introduce pfctl_pool

    Introduce pfctl_pool to be able to extend the pool part of the pf rule
    without breaking the ABI.

    Reviewed by:    kp
    MFC after:      4 weeks
    Differential Revision:  https://reviews.freebsd.org/D29721

As @0mp pointed out, with a random patch from Bugzilla you would commit with git commit --author="Bob Snow <bob@snow.us>" to get the correct authorship recorded and you would be reflected on the "Commit" line (this requires --pretty=full to show). Going forward, we should be encouraging git show style patches so that you can just git am it and not worry about the metadata or commit message.

0mp edited the summary of this revision. (Show Details)

Update MFH description

Why remove Relnotes? It would be great if people could use it for important stuff, could be used when branching a new quarterly branch, or when a release is cut.

As @0mp pointed out, with a random patch from Bugzilla you would commit with git commit --author="Bob Snow <bob@snow.us>" to get the correct authorship recorded and you would be reflected on the "Commit" line (this requires --pretty=full to show). Going forward, we should be encouraging git show style patches so that you can just git am it and not worry about the metadata or commit message.

Aha! That clicked for me. Thanks!

In D29861#670208, @mat wrote:

Why remove Relnotes? It would be great if people could use it for important stuff, could be used when branching a new quarterly branch, or when a release is cut.

I removed it because I always found it confusing in the ports world. We don't really use it consistently, sometimes committers put links to port's release notes there. Also, the only valid value for this metadata field is yes according both to the commit hook and the Committer's Guide, which makes no sense in terms of ports because we have no ports releases.

That's my reasoning.

If we can benefit from a field where we can mark important events in the ports tree then I'd introduce a new metadata field. This way we avoid confusion with how this field works in the src tree.

sbz added inline comments.
.hooks/prepare-commit-msg
59 ↗(On Diff #87803)

I think we can update<repo> and set it to be ports here, now it lives in the repo.

0mp edited the summary of this revision. (Show Details)
  • Add Relnotes back as requested by @mat.
  • Update the Pull Request field
0mp marked an inline comment as done.Apr 22 2021, 8:16 AM
This revision was not accepted when it landed; it landed in state Needs Review.Apr 22 2021, 9:52 AM
Closed by commit R11:37dfa4eeea6d: git hooks: Adjust hooks for the ports tree (authored by 0mp, committed by mat). · Explain Why
This revision was automatically updated to reflect the committed changes.