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

croodle: init at 0.6.2 #242287

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

croodle: init at 0.6.2 #242287

wants to merge 1 commit into from

Conversation

jboynyc
Copy link
Contributor

@jboynyc jboynyc commented Jul 8, 2023

Description of changes

Add package for the croodle web app, a privacy-respecting scheduling and polling app.

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.11 Release Notes (or backporting 23.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.

@jboynyc jboynyc force-pushed the croodle branch 2 times, most recently from b9fda7d to 6fe0873 Compare July 10, 2023 08:13

in

stdenvNoCC.mkDerivation rec {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think the derivation arguments should be moved into the attribute set as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please elaborate?

Copy link
Contributor Author

@jboynyc jboynyc Jul 10, 2023

Choose a reason for hiding this comment

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

Sure thing. Right now, the dataDir and debug options can be overridden because they appear in the derivation argument set:

{ lib
, fetchurl
, nixosTests
, stdenvNoCC
, writeText
, dataDir ? "/var/lib/croodle"
, debug ? false
}:

let
  configFile = ...
in
stdenvNoCC.mkDerivation (finalAttrs: { ...

If I'm understanding the finalAttrs pattern, I could also do the following:

{ lib
, fetchurl
, nixosTests
, stdenvNoCC
}:

stdenvNoCC.mkDerivation (finalAttrs: { ...
  dataDir = "/var/lib/croodle";
  debug = false;

Then I could access the values of finalAttrs.dataDir and finalAttrs.debug during the patch phase to write the configuration file. They would still be overrideable because of how the derivation wrapped in a lambda works.

Is my understanding correct and would you suggest I do so?

pkgs/servers/web-apps/croodle/default.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/croodle/default.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/croodle/default.nix Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants