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

argos: unstable-20230404 → unstable-2023-09-26 #274134

Merged
merged 1 commit into from
Dec 19, 2023
Merged

Conversation

andersk
Copy link
Contributor

@andersk andersk commented Dec 14, 2023

Description of changes

Required for GNOME 45 support.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Dec 14, 2023
@andersk andersk added 8.has: package (update) This PR updates a package to a newer version backport release-23.11 labels Dec 14, 2023
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Dec 14, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3103

@andersk andersk changed the title argos: 20230404 → 20230926 argos: 20230404 → 2023-09-26 Dec 17, 2023
@pbsds
Copy link
Member

pbsds commented Dec 18, 2023

please reword the commit gnomeExtensions.argos: 20230404 -> unstable-2023-0-26

Result of nixpkgs-review pr 274134 run on x86_64-linux 1

1 package built:
  • gnomeExtensions.argos

Required for GNOME 45 support.

Signed-off-by: Anders Kaseorg <[email protected]>
@andersk
Copy link
Contributor Author

andersk commented Dec 18, 2023

The previous version was equally “unstable” (note the pname change), in that there are no tagged releases at all. Amended.

@andersk andersk changed the title argos: 20230404 → 2023-09-26 argos: unstable-20230404 → unstable-2023-09-26 Dec 18, 2023
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

there is still a special symbol arrow in the commit message but we don't want to be to picky, do we?

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 19, 2023
@piegamesde
Copy link
Member

@SuperSandro2000 since when are we against these? I am happy to use them and happy for every other person who does.

@piegamesde piegamesde merged commit 2f1a960 into NixOS:master Dec 19, 2023
20 checks passed
Copy link
Contributor

Successfully created backport PR for release-23.11:

@SuperSandro2000
Copy link
Member

@SuperSandro2000 since when are we against these? I am happy to use them and happy for every other person who does.

I don't know how to type those on a normal keyboard and I don't think I am alone with that. Also on mobile the standard special character keyboard doesn't contain it.
This is just a minor inconvenience when working with those commits (e.g. grepping though the history) but still makes things yet a little more complex.

Also I wouldn't be surprised if in unicode style there are at least two of those errors which are displayed very similar.

@pbsds
Copy link
Member

pbsds commented Dec 19, 2023

Many $EDITORs seems to turn -> into , judging by the nixpkgs git log. I consider that a lost battle and tooling will simply have to adapt.

The gnomeExtensions. prefix on the other hand was totally missed /shrug

@piegamesde
Copy link
Member

@pbsds good catch, and sorry for that

@pbsds
Copy link
Member

pbsds commented Dec 19, 2023

Eh, i consider the git messages at best a heuristic of what is actually affected by the change. All tooling that parse the commit titles must deal with a lot of fuzz and provide multiple fallbacks

They do however drive the thought process on how to split commits into distinct atomic changes, which did not fail in this PR.

@piegamesde
Copy link
Member

At least getting the package name right is important though, as OfBorg will use it to know which package to build.

@andersk
Copy link
Contributor Author

andersk commented Dec 19, 2023

Clearly ofborg is not parsing the commit message for that. It labelled this “10.rebuild-linux: 1” without the help of a gnomeExtensions hint.

@andersk andersk deleted the argos branch December 19, 2023 18:36
@SuperSandro2000
Copy link
Member

Many $EDITORs seems to turn -> into ,

Which ones? How is that feature called? I am actually interested in that feature but I've never seen this before.

@piegamesde
Copy link
Member

@andersk that's a different bit of logic, which indeed does not read the commit message. If you look at the CI for the commit, you'll see that it didn't build the package, which it otherwise would have

@andersk
Copy link
Contributor Author

andersk commented Dec 19, 2023

Mm okay, I do see that. (On the other hand, why? It seems like reusing the evaluation it already does would be way more robust than scraping commit messages, not to mention more scalable than spending reviewers’ limited available time checking thousands of PRs for commit message standardization. Maybe a better topic for elsewhere. I’ll keep this in mind for now.)

@piegamesde
Copy link
Member

Yeah, I'll redirect you to the true experts in #ofborg:nixos.org on matrix

@pbsds
Copy link
Member

pbsds commented Dec 20, 2023

Which ones? How is that feature called? I am actually interested in that feature but I've never seen this before.

Baseless guess on my part, but there has to be something that does it, because it is so common.

@SuperSandro2000
Copy link
Member

Baseless guess on my part, but there has to be something that does it, because it is so common.

Then I have never come across this and actually I don't think it is common that people use this in their commit titles. Most of them are just normal asci chars.

@pbsds
Copy link
Member

pbsds commented Dec 20, 2023

$ git log --oneline | grep "→" | wc -l
5878

Sorry for all the pings OP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants