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

geogebra: make language configurable via config.nix #27766

Closed
wants to merge 1 commit into from
Closed

geogebra: make language configurable via config.nix #27766

wants to merge 1 commit into from

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jul 30, 2017

Motivation for this change

I think it might be way easier to configure geogebra using config.nix rather than building an overlay to pass the modified language parameter to the derivation.

Things done

Please check what applies. Note that these are not hard requirements but merely serve as information for reviewers.

  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@FRidh
Copy link
Member

FRidh commented Jul 30, 2017

I think it might be way easier to configure geogebra using config.nix rather than building an overlay to pass the modified language parameter to the derivation.

You can just override the derivation. Whether you put the override in a overlay or directly in your config.nix you can decide.

The config set is for configuration of the Nixpkgs package set. Configuration of individual packages should not be in there, at least not in my opinion.

@Ma27
Copy link
Member Author

Ma27 commented Jul 31, 2017

thanks for your response.

There are two reasons why I implemented this change:

  • it feels (at least for me, but I can be wrong) way more convenient to adjust configuration parameters using the config.nix rather than overriding derivations to modify parameters.
  • I've seen this pattern used by several other packages (e.g. aegisub, avahi, blueman or even chromium) so I assumed this is a common approach :-)

@FRidh
Copy link
Member

FRidh commented Jul 31, 2017

it feels (at least for me, but I can be wrong) way more convenient to adjust configuration parameters using the config.nix rather than overriding derivations to modify parameters.

I think it is in principle fine to provide functions in Nixpkgs. You could have a function there that takes the arguments you want to present and build a derivation from that. This is often not done because, while it could be more convenient from a UI point of view, it requires more effort to create and maintain such a layer, while in the end it will be hardly used. This is also why it is typically recommended to use overrideAttrs to override build flags, instead of us presenting every possible option as a function parameter.

I've seen this pattern used by several other packages

Unfortunately, yes...

@Ma27
Copy link
Member Author

Ma27 commented Aug 1, 2017

Thanks for the clarification @FRidh.

As I understand the original intent of config.nix I think it's better to close this for now :-)

@Ma27 Ma27 closed this Aug 1, 2017
@Ma27 Ma27 deleted the geogebra/allow-configuration-through-config branch August 1, 2017 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants