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

Jump to bottom

zenmonitor: init at 1.3 #73815

Merged
merged 1 commit into from Jan 29, 2020
Merged

zenmonitor: init at 1.3 #73815

merged 1 commit into from Jan 29, 2020

Conversation

alexbakker
Copy link
Member

Motivation for this change

Monitoring software for AMD Zen-based CPUs. See also: #73814.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@alexbakker
Copy link
Member Author

@erikarvstedt Thanks. Fixed.

@alexbakker
Copy link
Member Author

@erikarvstedt Done.

erikarvstedt
erikarvstedt approved these changes Dec 7, 2019
View reviewed changes
Copy link
Member

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

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

ACK 900e94f40fd783a242c9117eecd578df3fe1d4ca

@alexbakker
Copy link
Member Author

What's the status here? Can this be merged?

@alexbakker
Copy link
Member Author

@jtojnar Sure. Done.

@Br1ght0ne
Copy link
Member

Br1ght0ne commented Jan 16, 2020 •
edited

I think the kernel doesn't provide cpuid.h for aarch64. Maybe adding llvmPackages.libclang to nativeBuildInputs can resolve this?

Or we can just mark aarch64 as broken.

@alexbakker
Copy link
Member Author

@filalex77 I've restricted the list of platforms to i686-linux and x86_64-linux.

@alexbakker
Copy link
Member Author

@marsam This PR appears to be stuck.

marsam
marsam reviewed Jan 27, 2020
View reviewed changes
Copy link
Contributor

@marsam marsam left a comment

Choose a reason for hiding this comment

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

Thanks for the heads up, I left a few comments; besides that, LGTM

pkgs/os-specific/linux/zenmonitor/default.nix Outdated Show resolved Hide resolved
pkgs/os-specific/linux/zenmonitor/default.nix Outdated Show resolved Hide resolved
pkgs/os-specific/linux/zenmonitor/default.nix Show resolved Hide resolved
@alexbakker alexbakker force-pushed the zenmonitor branch 2 times, most recently from f1b286c to 3513858 Compare January 29, 2020 08:57
@alexbakker alexbakker changed the title zenmonitor: init at 1.2 zenmonitor: init at 1.3 Jan 29, 2020
@alexbakker
Copy link
Member Author

@marsam Bumped to 1.3 and addressed your comments.

@alexbakker
Copy link
Member Author

@marsam Done.

@marsam marsam merged commit 90d969e into NixOS:master Jan 29, 2020
@marsam
Copy link
Contributor

marsam commented Jan 29, 2020

@alexbakker thank you for your contribution

@alexbakker
Copy link
Member Author

@marsam Thanks for your help!

anna328p pushed a commit to anna328p/nixpkgs that referenced this pull request Feb 2, 2020
@marsam @anna328p
Merge pull request NixOS#73815 from alexbakker/zenmonitor
19136d2 
zenmonitor: init at 1.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marsam marsam marsam left review comments

@jtojnar jtojnar jtojnar left review comments

@Br1ght0ne Br1ght0ne Br1ght0ne approved these changes

@erikarvstedt erikarvstedt erikarvstedt approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@alexbakker @Br1ght0ne @marsam @jtojnar @erikarvstedt