Skip to content

zeal: migrate to by-name#358295

Closed
AndersonTorres wants to merge 1 commit intoNixOS:masterfrom
museoa:upload-zeal
Closed

zeal: migrate to by-name#358295
AndersonTorres wants to merge 1 commit intoNixOS:masterfrom
museoa:upload-zeal

Conversation

@AndersonTorres
Copy link
Member

And merge the expressions in a convoluted data structure.

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

@AndersonTorres AndersonTorres marked this pull request as ready for review November 23, 2024 02:42
@AndersonTorres
Copy link
Member Author

@ofborg build zeal-qt6 zeal-qt5

And merge the expressions in a convoluted data structure.
@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/4870

@teto
Copy link
Member

teto commented Nov 23, 2024

this looks convoluted and having both qt5 and qt6 as args looks odd.
The doc recommands this approach https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/README.md#recommendation-for-new-packages-with-multiple-versions

@teto
Copy link
Member

teto commented Nov 23, 2024

so my link was for different software versions but maybe it applies here too. Why do we need to keep both qt5 and qt6 variants anyway ?
would it be simpler to do this from all-packages.nix ?

zeal = callPackage ../byname/zeal { qt = qt6 }
zeal-qt5 = zeal.override {qt=qt5}` 

@ofborg ofborg bot requested a review from peterhoeg November 23, 2024 22:06
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 23, 2024
@AndersonTorres
Copy link
Member Author

this looks convoluted

More or less, but I think we have some precedent on Nixpkgs. Emacs uses a similar style.

and having both qt5 and qt6 as args looks odd.

I think this is not unusual in general.
Many packages accept multiple GUI backends, and we put all of them as arguments.

Why do we need to keep both qt5 and qt6 variants anyway ? would it be simpler to do this from all-packages.nix ?

callPackage is a bit temperamental.

#324199 (comment)

And I am trying to circumvent this.

@AndersonTorres AndersonTorres deleted the upload-zeal branch December 14, 2024 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants