Skip to content

issue/pr template: stop recommending the usage of git-blame to find people to CC #7106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sudoforge opened this issue May 21, 2025 · 2 comments
Assignees

Comments

@sudoforge
Copy link
Contributor

sudoforge commented May 21, 2025

... when creating an issue or a pull request, there's text telling users to ping meta.maintainers or to fall back to git blame and to ping people you find, even though this is a terrible way to find out who potentially owns a module/file.

- type: textarea
attributes:
label: Maintainer CC
description: |
Please @ people who are in the `meta.maintainers` list of the offending module.
If in doubt, check `git blame` for whoever last touched something.

"who last touched something" is potentially some random drive-by contributor who fixed a typo.

Originally posted by @sudoforge in #7105


we should change this in both the issue and pull request templates. a better heuristic to determine who might be maintaining a module, or who owns a module, is git shortlog -sne --since '1 year ago' -- <path...>. the date provided to --since is arbitrary; make it 2, 5, 10, whatever. we can tell them to ping the top ~3-5 people. or not.

the point is that this would show a list of users, ordered by the number of commits they've made to the provided path(s). i find this to be far superior to determining who the maintainer of something without meta.maintainers might be, and is also likely easier for ad-hoc contributors/users to use and figure out.

git-blame is useful if a user has a specific issue and has done the work to track it down to a given set of lines, but even then, it falls short because of (usually unrelated) tree-wide changes that might touch one or more lines (e.g. formatting, moving files, etc). used without exact intention (as is done currently), it generates spurious pings to users who do not own something.

we're all busy, let's reduce the noise :)

@donottellmetonottellyou

I will post this here because I think it is useful context.

my first comment is abstract, and not directed at you.

Understood, I was only providing context as to why I fell back to using blame in the first place.

it's a statement about the fact that when creating an issue or a pull request, there's text telling users to ping meta.maintainers or to fall back to git blame and to ping people you find, even those this is a terrible way to find out who potentially owns a module/file.
"who last touched something" is potentially some random drive-by contributor who fixed a typo. case in point, you found my name and pinged me because i added a single new option to this module, and apparently, those lines haven't been touched yet.

I agree that this is not ideal, and falls back to the general nixpkgs/home-manager policy that "maintainers don't have to commit everything". This policy is great for ensuring that a maintainer's vacation won't completely bork a package for several weeks, but is an issue if we have to rely on git blame (or in my case, the Github commit history) to check for who is responsible for fixing an issue.

I completely agree that if a change is irrelevant, getting pinged is probably a waste of your time, especially if you make a habit of making widespread QOL changes to nixpkgs/home-manager

In addition, even if we were to keep the current policy, it would likely be a good idea to specify that you should look at the master branch blame history. I realized 24.11 hasn't had any zsh changes for 9 months! You only got pinged because I was on stable.

if your issue was about those lines, i'd have no problem being pinged. but that isn't what your issue is about, and that is what i'm pointing out is problematic.

I also think it is discouraging to users of nix to have to triage the git history of a package they know nothing about to report an issue they have with the package. At the very least, a list of maintainers should be in the same file as the module, to make reporter's jobs easier. I don't think it should be necessary for every user of home-manager to be expected to have a local git clone to run more complicated git commands either. That's the job of a maintainer!

I do agree that due diligence should be done to determine a commit is causing a problem before pinging the person who made the commit.

@sudoforge
Copy link
Contributor Author

I also think it is discouraging to users of nix to have to triage the git history of a package they know nothing about to report an issue they have with the package. At the very least, a list of maintainers should be in the same file as the module, to make reporter's jobs easier. I don't think it should be necessary for every user of home-manager to be expected to have a local git clone to run more complicated git commands either. That's the job of a maintainer!

well, this is touching on a much larger problem - we don't have maintainers defined for every file, and that is both a process and people problem (enforcing that everything is owned by someone is difficult if you don't have enough people).

but, yes, in an ideal world, everything would have one or more owners, and ideally, they would be listed in meta.maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants