-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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
Sort the maintainer-list #77784
Sort the maintainer-list #77784
Conversation
Update: this splits all the capitalized people from their lowercase friends. I'll investigate having is sort in a case-insensitive way. |
53ab7d6
to
58631c4
Compare
Update: I have resorted (to) using |
I'm not sure if it's worth doing this. I think either we have a check that prevents people from evaluating it when it's unsorted, or we don't keep it sorted. I personally don't see the reason behind keeping it sorted, so I'd prefer the latter. |
|
Wouldn't this cause merge conflicts for each PR which adds a new maintainer? Apart from that, I agree with @infinisil: any decent editor should've some kind of search feature, finding the appropriate location for a new attribute takes IMHO way more time. |
I think it should have been sorted in the first place, with a CI check. But that's just personal opinion. I don't like it when things are... disorderly. But this is all just bikeshedding anyway. |
This is unlikely to work with a file changed this often, for the same reason all-packages.nix can’t be sorted. You can test that it’s sorted at the HEAD of the PR, but unless you want to rerun CI against the merge for every PR that touches this file every time any commit is added to master that touches it, it will still get unsorted when merged. |
@alyssais not sure the situation you describe can happen without causing a merge conflict. |
@alyssais not sure the situation you describe can happen without
causing a merge conflict.
You might be right... How come all-packages.nix is the way it is, then?
|
> @alyssais not sure the situation you describe can happen without
> causing a merge conflict.
You might be right... How come all-packages.nix is the way it is, then?
1. Group by topic, in a way that is similar but slightly different from the directory structure.
2. Get some entries that are close to each other because they make so much sense together (at some point it was clang+llvm when there was only initial support, I think).
3. Make re-sorting prohibitively expensive because of merge conflicts and churn.
4. Observe that the file is so large people just search and insert if it (locally) looks like the correct place.
…
|
I'd also say its not worth doing it, at least not in a one-off fashion. If we would introduce CI to keep this from happening in the future at the same time, that might be worth thinking about. But in its current state it will just make |
Since this has stalled for a week and most people seem to be on the "not worth it" side, do you agree that we can close this @yorickvP? |
CI for not making it worse seems nontrivial. Closing for now. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
The maintainer-list has a comment about keeping it sorted. Sadly, it still got unsorted.
I have sorted it using
nix eval '(import <nixpkgs/maintainers/maintainer-list.nix>)' | nixfmt
, so that we can all sleep soundly again.cc: @grahamc
TODO: investigate impact on maintainer scripts running git blame on this list.