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

nixos/dovecot2: turn mailboxes-option into an attr-set #89486

Merged
merged 2 commits into from Jun 17, 2020
Merged

nixos/dovecot2: turn mailboxes-option into an attr-set #89486

merged 2 commits into from Jun 17, 2020

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jun 4, 2020

Motivation for this change

  • 7922a78: add the autoexpunge-setting to services.dovecot2.mailboxes. With this option specified, email older than the specified time will be automatically removed.

    This is e.g. helpful for spam-folders.

  • 907d046 : turned the mailboxes-option into an attr-set. This makes it easier to override the settings for a single mailbox.

    It's still possible to use a list for backwards-compatibility, but this will raise a deprecation warning.

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.

@Ma27 Ma27 mentioned this pull request Jun 5, 2020
Closed
10 tasks
@Ma27 Ma27 requested review from flokli, cole-h and aanderse and removed request for flokli and cole-h June 10, 2020 17:43
@Ma27
Copy link
Member Author

Ma27 commented Jun 10, 2020

As this didn't get any feedback until now, I decided to request a review from the folks suggested by GitHub :)

cole-h
cole-h approved these changes Jun 10, 2020
View reviewed changes
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

Small nit, else LGTM. Though, I don't use dovecot, so I can't verify this works... I'm trusting you already did that ;)

nixos/modules/services/mail/dovecot.nix Outdated Show resolved Hide resolved
@Ma27
Copy link
Member Author

Ma27 commented Jun 10, 2020

Fixed, thanks!

Just realized that GitHub also suggested reviewers that only commited changes to rl-2009.xml and not to the dovecot2-module, so sorry about the unnecessary notification!

Though, I don't use dovecot, so I can't verify this works... I'm trusting you already did that ;)

Just for the record, I'm using those changes on my personal mailserver for about a week now without any problems.

@cole-h
Copy link
Member

cole-h commented Jun 10, 2020

All good -- I'm always happy to review stuff! (Though if it means I need to build/run something that I don't have an interest in or already use, it usually falls to the bottom of my ever-expanding TODO list...)

@Ma27
Copy link
Member Author

Ma27 commented Jun 16, 2020

Rebased onto latest master.

nlewo
nlewo reviewed Jun 17, 2020
View reviewed changes
Copy link
Member

@nlewo nlewo left a comment

Choose a reason for hiding this comment

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

👍 for this change.

I think we will remove the list format for the release 21.03. What about announcing this date in the warning, release notes and adding a comment close to this code?

nixos/doc/manual/release-notes/rl-2009.xml Outdated Show resolved Hide resolved
@Ma27
nixos/dovecot2: refactor mailboxes option
e826a6c 
Specifying mailboxes as a list isn't a good approach since this makes it
impossible to override values. For backwards-compatibility, it's still
possible to declare a list of mailboxes, but a deprecation warning will
be shown.
@Ma27
Copy link
Member Author

Ma27 commented Jun 17, 2020

@nlewo fixed :)

@nlewo
Copy link
Member

nlewo commented Jun 17, 2020

@GrahamcOfBorg test dovecot

@nlewo
Copy link
Member

nlewo commented Jun 17, 2020

Thanks!

@nlewo nlewo merged commit b20f911 into NixOS:master Jun 17, 2020
@Ma27 Ma27 deleted the dovecot-mailboxes branch June 17, 2020 20:20
@infinisil infinisil mentioned this pull request Aug 18, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nlewo nlewo nlewo approved these changes

@cole-h cole-h cole-h approved these changes

@flokli flokli Awaiting requested review from flokli

@aanderse aanderse Awaiting requested review from aanderse

@WilliButz WilliButz Awaiting requested review from WilliButz

@lheckemann lheckemann Awaiting requested review from lheckemann

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@Ma27 @cole-h @nlewo