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.

Sign up for GitHub

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

Jump to bottom

ckb/ckb-next: 0.2.9 -> 0.3.2, and cleanup #40043

Merged
merged 7 commits into from Nov 9, 2018
Merged

ckb/ckb-next: 0.2.9 -> 0.3.2, and cleanup #40043

merged 7 commits into from Nov 9, 2018

Conversation

kierdavis
Copy link
Contributor

@kierdavis kierdavis commented May 6, 2018 •
edited

  • updated ckb package from 0.2.9 to 0.3.0 (EDIT: now 0.3.2)
  • renamed package and module from "ckb" to "ckb-next", following the move by the upstream developers to do the same
  • minor tidy-ups
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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.

samueldr
samueldr requested changes Aug 8, 2018
View reviewed changes
Copy link
Member

@samueldr samueldr left a comment

Choose a reason for hiding this comment

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

Thanks!

Minor changes requested, see the comments. Other than those nitpicks it looks like an approval from me.

I would additionally like if you could add the option rename to the release-notes (nixos/doc/manual/release-notes/rl-1809.xml). If you can't or don't want to write the release notes just ask and I'll handle it.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/ckb-next/default.nix Outdated Show resolved Hide resolved
@kierdavis kierdavis force-pushed the ckb-update-and-cleanup branch 2 times, most recently from b3dca7a to d30ff2f Compare October 21, 2018 17:05
@kierdavis
ckb-next: 0.2.9 -> 0.3.2
85526bc 
In this update:

* binaries `ckb` and `ckb-daemon` are renamed to `ckb-next` and `ckb-next-daemon`
* build system changed from qmake to cmake
* the directory searched for animation plugins no longer needs to be patched, as a result of the build system change
* modprobe patch has been bumped, since the source repository layout has changed
* the cmake scripts are quite FHS-centric and require patching to fix install locations
@kierdavis
ckb module: update systemd service parameters to match upstream
8069b09 
This changes the description and restart mode to the values present
in lib/systemd/system/ckb.service within the ckb package.
@kierdavis
ckb, ckb module: rename to ckb-next
8117878 
The upstream package has officially changed its name to ckb-next.
@kierdavis
ckb module: use exec when starting the daemon process
dfdaf39 
This avoids leaving the parent shell process (the one executing the
unit script) lying around.
@kierdavis
ckb-next: rename ckb-modprobe.patch to modprobe.patch
7a0d849 
This keeps the naming consistent with the other two patches in
this directory.
@kierdavis
Copy link
Contributor Author

@samueldr I've pushed an updated version of this PR. Changes are:

  • Upgraded to version 0.3.2
  • Rebased onto master to fix merge conflicts
  • Moved alias to pkgs/top-level/aliases.nix as requested
  • Added an entry to the 19.03 release notes

P.S. Github's UI appears to be behaving weirdly for me: on some page loads, the head commit of kierdavis/ckb-update-and-cleanup is wrong, resulted in outdated logs/diffs being shown as if they are current. When reviewing please double-check that the most recent commit on the branch is 7a0d8496537a8b31a0697a8d08fa042837fb467c.

@kierdavis kierdavis changed the title ckb/ckb-next: 0.2.9 -> 0.3.0, and cleanup ckb/ckb-next: 0.2.9 -> 0.3.2, and cleanup Oct 22, 2018
@kierdavis kierdavis mentioned this pull request Oct 22, 2018
Closed
9 tasks
samueldr
samueldr reviewed Nov 6, 2018
View reviewed changes
nixos/modules/hardware/ckb-next.nix Outdated
imports = [
(mkRenamedOptionModule ["hardware" "ckb" "enable"] ["hardware" "ckb-next" "enable"])
(mkRenamedOptionModule ["hardware" "ckb" "package"] ["hardware" "ckb-next" "package"])
];
Copy link
Member

Choose a reason for hiding this comment

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

Oops, looks like I missed those renames. I personally don't mind if those are kept local to the relevant module, but it looks like most renames are in nixos/modules/rename.nix.

I wouldn't hold back the change on account of that, though let's say what you yourself think about it, and if others have a strong opinion about that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking it would be more consistent to put them in nixos/modules/rename.nix. I feel it makes sense for that file to be the first port of call when (if?) deprecated aliases for options are eventually cleaned up.

samueldr
samueldr approved these changes Nov 6, 2018
View reviewed changes
Copy link
Member

@samueldr samueldr left a comment

Choose a reason for hiding this comment

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

Other than the really nit-picky comment, and the merge conflict, this looks fine. Everything fixed as asked.

@kierdavis
Copy link
Contributor Author

@samueldr Thanks for the review! I've fixed the merge conflict and moved the option renames as suggested.

@samueldr samueldr merged commit 2f668e3 into NixOS:master Nov 9, 2018
@kierdavis
Copy link
Contributor Author

Thanks!

@kierdavis kierdavis deleted the ckb-update-and-cleanup branch November 11, 2018 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samueldr samueldr samueldr approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: clean-up 8.has: documentation 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kierdavis @samueldr @GrahamcOfBorg