Skip to content
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

maintainers/scripts/update.nix: Remove unicode from message #224746

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

viraptor
Copy link
Contributor

@viraptor viraptor commented Apr 5, 2023

Remove unicode from message and comply with CONTRIBUTING.md:

Format the commit messages in the following way:

(pkg-name | nixos/<module>): (from -> to | init at version | refactor | etc)

It's not used frequently and the ascii version is nicer (because it's type-able regardless of layout) / expected (because it's documented) for automatic parsing.

git log --grep '->' --oneline --since '2022-01-01' | wc -l
   67791
git log --grep '→' --oneline --since '2022-01-01' | wc -l
    2914
Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

…y with CONTRIBUTING.md

It's not used infrequently and the ascii version is nicer for automatic parsing.
```
git log --grep '->' --oneline --since '2022-01-01' | wc -l
   67791
git log --grep '→' --oneline --since '2022-01-01' | wc -l
    2914
```
@viraptor viraptor requested a review from jtojnar as a code owner April 5, 2023 01:37
@viraptor viraptor mentioned this pull request Apr 5, 2023
12 tasks
@andersk
Copy link
Contributor

andersk commented Apr 5, 2023

the ascii version is nicer / expected for automatic parsing

You’re entitled to your opinion. My opinion is that the Unicode version is nicer, and automatic parsers should conform to the commit messages we actually have. (I’m willing to be overruled, but I think it’s important to note that our opinions differ.)

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Apr 5, 2023
Copy link
Member

@Artturin Artturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> is the dominant one and used in CONTRIBUTING.md

It's also easy to type and minor aesthetics don't matter as we're not in a gallery

@Artturin
Copy link
Member

Artturin commented Apr 5, 2023

#155309 (comment)

One issue with switching update.nix to use ASCII is that update.nix is also used by repos which do not allow the ASCII variant, whereas Nixpkgs seems to have been tolerating the Unicode variant so far.

What repos are those

@Artturin Artturin requested a review from SuperSandro2000 April 5, 2023 02:19
@viraptor
Copy link
Contributor Author

viraptor commented Apr 5, 2023

automatic parsers should conform to the commit messages we actually have

As long as it's clear that we use that format, that's right. Currently, anyone who writes some one-off script without specifically knowing about this difference (e.g. me) will use the existing docs and expect ->. They'll also tell people they don't comply with CONTRIBUTING if they notice a different format.

To be clear, I'm not opposed to the unicode symbol being used. I'm proposing a bug fix where we document one format, but do something different.

But the current state is broken. If we explicitly wants to use unicode, then we should make a different bug fix which:

  • updates CONTRIBUTING.md
  • updates the docs
  • (optionally) changes the defaults in other scripts, because having inconsistent approaches means the parsing has to deal with (->|→)

Also, from the other comment:

it is as simple as pressing Compose followed by those two characters.

That really depends on the system / layout / os, etc. I don't have the compose key, so when discussing this, I need to keep copy-pasting the symbol.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Apr 5, 2023

My opinion is that the Unicode version is nicer, and automatic parsers should conform to the commit messages we actually have.

but it isn't and this is the quickest fix to get it working and things inline with contributing. If ofborg and so on support the unicode character, we can revisit this.

Also side note: I don't even know how to type the unicode version on my keyboard.

@SuperSandro2000 SuperSandro2000 merged commit 10978e9 into NixOS:master Apr 5, 2023
@viraptor viraptor deleted the asciify-message branch April 5, 2023 09:41
@jtojnar
Copy link
Member

jtojnar commented Apr 5, 2023

What repos are those

I use update.nix in my personal repos and do not want ugly “pretend arrows” there.

CONTRIBUTING.md is not a formal specification and commit messages are not written in a formal language. The only requirement is that the commit messages roughly match the described shape and Unicode arrows do just fine in that regard. That is why they are generally accepted and any tool that wants to attempt to parse historic commit messages will quickly discover that it needs to support them.

@SuperSandro2000
Copy link
Member

That is why they are generally accepted

To my knowledge they are not accepted.

any tool that wants to attempt to parse historic commit messages

This was mostly about current new commit messages as eg ofborg is not parsing historic data.

@7c6f434c
Copy link
Member

7c6f434c commented Apr 5, 2023

That is why they are generally accepted and any tool that wants to attempt to parse historic commit messages will quickly discover that it needs to support them.

It is generally accepted that some share of commit messages do not follow CONTRIBUTING.md precisely. Getting to zero is hard, but if a safe change in a script can reduce the amount of deviations, that's a nice thing.

@toastal
Copy link
Contributor

toastal commented Apr 5, 2023

A lot of those -> commits are coming from the update bots without human involvement and could be tweaked either way. +1 for Unicode support; Unicode lets you mean what you say.

I don't even know how to type the unicode version on my keyboard.

If you commit via the CLI git commit will likely bring you into a Vim-compatible editor to format your commit message. The diagraph is ->, or with default bindings <C-k>->, or Ctrl+k then - then >.

@SuperSandro2000
Copy link
Member

TIL

The diagraph is ->, or with default bindings <C-k>->, or Ctrl+k then - then >.

You can do 99% of your work with 1% of vim's features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants