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

zenith: init at 0.8.0 #83183

Merged
merged 1 commit into from Apr 4, 2020
Merged

zenith: init at 0.8.0 #83183

merged 1 commit into from Apr 4, 2020

Conversation

bbigras
Copy link
Contributor

@bbigras bbigras commented Mar 23, 2020

Motivation for this change
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 nixpkgs-review --run "nixpkgs-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.

/nix/store/1x1i6fsqfgs7sa3934imxx1wsclxa2aw-zenith-0.7.5 34.6M

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/134

@evils
Copy link
Member

evils commented Mar 24, 2020

seems to work
too bad it doesn't have the equivalent of htop's p to remove leading paths (like /nix/store/....)

@bbigras
Copy link
Contributor Author

bbigras commented Mar 24, 2020

I made the change. Thanks.

bachp
bachp approved these changes Mar 25, 2020
View reviewed changes
Copy link
Member

@bachp bachp left a comment

Choose a reason for hiding this comment

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

Tested and working.

As a remark 0.7.6 was release in the meantime. I don't know if you want to update or do that later in a next PR.

@bbigras
Copy link
Contributor Author

bbigras commented Mar 25, 2020

As a remark 0.7.6 was release in the meantime. I don't know if you want to update or do that later in a next PR.

I updated it. Thanks! I forgot to subscribe to the releases notifications.

@bbigras bbigras changed the title zenith: init at 0.7.5 zenith: init at 0.7.7 Mar 27, 2020
@bbigras
Copy link
Contributor Author

bbigras commented Mar 27, 2020

Updated to 0.7.7.

@evils
Copy link
Member

evils commented Mar 30, 2020

if you can't fix the build on aarch64 you should probably adjust meta.platforms
(maybe to platforms.x86?)

@bbigras
Copy link
Contributor Author

bbigras commented Mar 30, 2020

Thanks. I made the change and I'll open a bug report.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/130

@bbigras bbigras changed the title zenith: init at 0.7.7 zenith: init at 0.8.0 Apr 2, 2020
timokau
timokau approved these changes Apr 4, 2020
View reviewed changes
Copy link
Member

@timokau timokau left a comment

Choose a reason for hiding this comment

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

Thanks. I made the change and I'll open a bug report.

For future reference, its always nice to link to those reports in source comments. I don't think that has to hold off the merge here though, this looks very high-quality and has already been sitting unmerged for too long :)

Thanks for the contribution and the reviews @bbigras, @evils-devils, @bachp!

@timokau timokau merged commit 696274f into NixOS:master Apr 4, 2020
@bbigras bbigras deleted the zenith branch April 4, 2020 16:25
@bbigras
Copy link
Contributor Author

bbigras commented Apr 4, 2020

For future reference, its always nice to link to those reports in source comments.

Thanks. I added a comment in the new PR #84260

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evils evils evils left review comments

@bachp bachp bachp approved these changes

@timokau timokau timokau approved these changes

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@bbigras @nixos-discourse @evils @bachp @timokau