Skip to content

fix: setup email list should only work with ACCOUNT_PROVISIONER=FILE#4453

Merged
polarathene merged 8 commits intomasterfrom
fix/setup-cli-email-list-add-compat-error
Apr 24, 2025
Merged

fix: setup email list should only work with ACCOUNT_PROVISIONER=FILE#4453
polarathene merged 8 commits intomasterfrom
fix/setup-cli-email-list-add-compat-error

Conversation

@polarathene
Copy link
Copy Markdown
Member

@polarathene polarathene commented Apr 23, 2025

Description

This addresses https://github.com/orgs/docker-mailserver/discussions/4452#discussioncomment-12918479

It should provide a better UX than a bunch of unhelpful errors like the user experienced.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes
  • I have added information about changes made in this PR to CHANGELOG.md

@polarathene polarathene added service/dovecot area/scripts kind/improvement Improve an existing feature, configuration file or the documentation labels Apr 23, 2025
@polarathene polarathene added this to the v15.1.0 milestone Apr 23, 2025
@polarathene polarathene self-assigned this Apr 23, 2025
Comment thread CHANGELOG.md Outdated
Comment thread target/bin/listmailuser
Comment on lines +11 to +13
if [[ ${ACCOUNT_PROVISIONER} != 'FILE' ]]; then
_exit_with_error "This command is only compatible with 'ACCOUNT_PROVISIONER=FILE'"
fi
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This could technically be dropped, but I don't think DMS is intended to support both file based accounts and LDAP at the same time, so I'm inclined to keep the early exit constraint here? 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fail-fast is the right approach here, indeed.

Comment thread target/scripts/startup/variables-stack.sh Outdated
Copy link
Copy Markdown
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

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

@georglauterbach
Copy link
Copy Markdown
Member

I pushed a fix for the still-occurring test failure. Hope the issue is now fixed.

@github-actions
Copy link
Copy Markdown
Contributor

Documentation preview for this PR is ready! 🎉

Built with commit: a871a58

@polarathene polarathene merged commit 491c30b into master Apr 24, 2025
9 checks passed
@polarathene polarathene deleted the fix/setup-cli-email-list-add-compat-error branch April 24, 2025 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/scripts kind/improvement Improve an existing feature, configuration file or the documentation service/dovecot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants