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

Postgrey #21586

Merged
merged 8 commits into from Jan 5, 2017
Merged

Postgrey #21586

merged 8 commits into from Jan 5, 2017

Conversation

gkleen
Copy link
Contributor

@gkleen gkleen commented Jan 2, 2017

Motivation for this change

The postgrey module was rather rudimentary

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
    • Linux
  • 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.

configuration.nix does not show socket nicely, I worked around that by making sure the default and example cover all options available in both submodules – is there a prettier solution?

@mention-bot
Copy link

@pngwjpgh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bendlas to be a potential reviewer.

@bendlas
Copy link
Contributor

bendlas commented Jan 2, 2017

The option renames will be painful for any existing users, but I admit that it makes slightly more sense, the way you did it. Hoping that I'm the only existing user, a tentative +1 from me.

@joachifm
Copy link
Contributor

joachifm commented Jan 2, 2017

You could add entries like those in https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/rename.nix to smoothe things out.

@gkleen
Copy link
Contributor Author

gkleen commented Jan 2, 2017

@joachifm @bendlas 9383b2c is untested.


Addendum: it works as expected with a new-style configuration at least

@joachifm
Copy link
Contributor

joachifm commented Jan 4, 2017

@bendlas are you okay with this? I have no stake in this module.

@bendlas
Copy link
Contributor

bendlas commented Jan 5, 2017

LGTM, thanks for the compat options!

@joachifm joachifm merged commit 02053c3 into NixOS:master Jan 5, 2017
@gkleen gkleen deleted the postgrey branch January 26, 2017 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants