Conversation
jtojnar
requested review from
FRidh,
cdepillabout,
jonringer,
mkg20001 and
sternenseemann
as code owners
github-actions
bot
added
6.topic: cinnamon
nixos/tests/all-tests.nix
Outdated
There was a problem hiding this comment.
Does the test name need to be updated in Hydra job? Or is nixos/release-combined.nix enough?
There was a problem hiding this comment.
This should be fine (as I see you updated the tested job, too). It will be like removing the gnome3 job and adding a gnome job; I don't think there's any way around that. Well, overall I rather see more potential costs from these mass "renamings" in code.
There was a problem hiding this comment.
What kinds of costs? We do have aliases.
There was a problem hiding this comment.
Say, various kinds of merge conflicts.
nixos/release.nix
Outdated
pkgs/desktops/gnome/default.nix
Outdated
There was a problem hiding this comment.
I wonder why we even had this.
There was a problem hiding this comment.
This is curious. This must have been always null.
There was a problem hiding this comment.
grilo has been in top-level for a while.
There was a problem hiding this comment.
Had to unref the links since the options have been renamed now but we should preserve the old options in old RNs.
There was a problem hiding this comment.
Also mentioned the rename in the current RN.
ofborg
bot
added
8.has: clean-up
|
I'm fine with renaming. I agree it's awkward to have gnome3 -> gnome40 |
Since GNOME version is now 40, it no longer makes sense to use the old attribute name.
ofborg
bot
removed
the
2.status: merge conflict
| }) {inherit (pkgs.gnome3) gnome-keyring; | ||
| }) {inherit (pkgs.gnome) gnome-keyring; |
There was a problem hiding this comment.
@sternenseemann Do we need to change something so that we don‘t override this?
There was a problem hiding this comment.
IIRC, it should just find the correct package somehow. https://github.com/NixOS/cabal2nix/blob/master/src/Distribution/Nixpkgs/Haskell/FromCabal.hs
There was a problem hiding this comment.
Yeah, looks like we are probably fine.
This got missed in the move (#122107) because it is in a “hidden” directory.
The rename was done long ago in NixOS/nixpkgs#122107 (back in 21.05), and since NixOS/nixpkgs#192681 the old `gnome3` name no longer works. Update the old virt-manager package accordingly.
The rename was done long ago in NixOS/nixpkgs#122107 (back in 21.05), and since NixOS/nixpkgs#192681 the old `gnome3` name no longer works. Update the old virt-manager package accordingly.
4 participants
Motivation for this change
Since GNOME version is now 40, it no longer makes sense to use the old attribute name.
cc @jonringer @vcunat @NixOS/gnome
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)