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

mozlz4a: enable build on Darwin #196021

Merged
merged 3 commits into from
Oct 15, 2022
Merged

mozlz4a: enable build on Darwin #196021

merged 3 commits into from
Oct 15, 2022

Conversation

pshirshov
Copy link
Contributor

See #196018

@pshirshov pshirshov requested a review from siraben October 14, 2022 17:06
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Oct 14, 2022
Copy link
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

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

Successfully decompresses my profile's search.json.mozlz4a.

Could you add yourself as a maintainer of this package for the Darwin part?

Result of nixpkgs-review pr 196021 run on aarch64-darwin 1

1 package built:
  • mozlz4a

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Oct 14, 2022
@ofborg ofborg bot requested a review from 7c6f434c October 14, 2022 17:17
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Oct 14, 2022
Copy link
Contributor

@kira-bruneau kira-bruneau left a comment

Choose a reason for hiding this comment

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

Thanks @pshirshov! Since I plan on maintaining the search portion of the firefox home-manager module, you can add me as a darwin maintainer if you don't want to maintain it yourself.

(I have a x86_64-darwin mac to test on)

@bobby285271 bobby285271 added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Oct 14, 2022
@ofborg ofborg bot requested a review from kira-bruneau October 15, 2022 12:38
@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Oct 15, 2022
@kira-bruneau kira-bruneau merged commit 6010f32 into NixOS:master Oct 15, 2022
@Atemu
Copy link
Member

Atemu commented Oct 15, 2022

@kira-bruneau please check new commit messages before merging. They do not fit the contributor guidelines.

@kira-bruneau
Copy link
Contributor

kira-bruneau commented Oct 15, 2022

@kira-bruneau please check new commit messages before merging. They do not fit the contributor guidelines.

Oh, sorry, I missed that! 😓 - I'm too used to people force-pushing their changes. I should be more careful.

@pshirshov pshirshov deleted the patch-3 branch October 15, 2022 13:50
@pshirshov
Copy link
Contributor Author

I think that squash on merge should be enabled for the repo.

@kira-bruneau
Copy link
Contributor

kira-bruneau commented Oct 15, 2022

@pshirshov I don't think we'd want to enable that, because in certain cases it makes sense to have multiple commits in one PR, and we don't want to lose that context.

The normal workflow in nixpkgs is to cleanup your history locally where it makes sense (eg. amend commits instead of adding "fixup" commits) and then force push your changes.

@ncfavier
Copy link
Member

Here's the comment where this was first explained to me: #104836 (comment)

@pshirshov
Copy link
Contributor Author

Yeah, but it's really convenient not to have local copy when I do some small changes in a nix formula. Also it's possible to allow squashes together with other policies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants