Skip to content

Consume self-deploy's startAt attribute#157883

Merged
sternenseemann merged 1 commit intoNixOS:masterfrom
wpcarro:master
Feb 3, 2022
Merged

Consume self-deploy's startAt attribute#157883
sternenseemann merged 1 commit intoNixOS:masterfrom
wpcarro:master

Conversation

@wpcarro
Copy link
Contributor

@wpcarro wpcarro commented Feb 2, 2022

As #157879 points-out, this attribute appears unused.

Fixes #157879

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 2, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 2, 2022
Copy link
Member

@tazjin tazjin left a comment

Choose a reason for hiding this comment

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

commit message should be changed to something like nixos/self-deploy: .... bla ...

As NixOS#157879 points-out, this attribute appears unused.

Fixes NixOS#157879
@lambdadog
Copy link
Contributor

lambdadog commented Feb 3, 2022

Good catch! Must have missed it when I rewrote the service for nixpkgs. I admit I'm happy to see actual use of the self-deploy service in the wild -- I've meant to write a blog post about it for ages but have never gotten around to it.

All looks good to me.

My personal preferences would prefer inherit (cfg) startAt; as I feel it makes it ever so slightly more transparent that the option is a direct passthrough to systemd.services.<name> for readers of the code but I'll be the first to admit that's just my own personal convention and I wouldn't be torn up if you prefer yours.

@lambdadog
Copy link
Contributor

Looking back on the code, it may also be worthwhile to make the minor change of gating the service's systemd dependency on cfg.switchCommand being "boot" if only for correctness' purpose.

@sternenseemann sternenseemann merged commit b830507 into NixOS:master Feb 3, 2022
wpcarro pushed a commit to wpcarro/nixpkgs that referenced this pull request Feb 3, 2022
wpcarro pushed a commit to wpcarro/nixpkgs that referenced this pull request Feb 3, 2022
sternenseemann pushed a commit that referenced this pull request Feb 4, 2022
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Oct 18, 2022
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Oct 18, 2022
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Dec 14, 2022
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jun 1, 2023
@Janik-Haag Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nixos/self-deploy: startAt option seems to be unused

5 participants