Skip to content

Add ability to build with Dovecot community repository#2797

Merged
casperklein merged 5 commits intodocker-mailserver:masterfrom
casperklein:dovecot
Sep 29, 2022
Merged

Add ability to build with Dovecot community repository#2797
casperklein merged 5 commits intodocker-mailserver:masterfrom
casperklein:dovecot

Conversation

@casperklein
Copy link
Copy Markdown
Member

@casperklein casperklein commented Sep 25, 2022

Description

This PR adds the ability, to use the latest Dovecot packages from the community repository. For this, the build argument DOVECOT_COMMUNITY_REPO=1 must be used.

The current version for Dovecot in the debian repository is 2.3.13. In the community repository it is 2.3.19.1.

There shouldn't be any problems for existing setups switching to the community repository, as the version gap is only minor. However to be safe, we can wait for the next DMS major release, to make the community repository the default.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change that does improve existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@casperklein casperklein added this to the v11.2.0 milestone Sep 25, 2022
@casperklein casperklein self-assigned this Sep 25, 2022
@casperklein casperklein requested a review from a team September 25, 2022 20:08
@casperklein casperklein added area/ci service/dovecot area/scripts kind/improvement Improve an existing feature, configuration file or the documentation labels Sep 25, 2022
@casperklein
Copy link
Copy Markdown
Member Author

casperklein commented Sep 25, 2022

Test 217 failed. I've restarted the test action.

This is the same test that failed in #2792

Edit: Tests were successful now.

@casperklein casperklein marked this pull request as ready for review September 25, 2022 21:11
@georglauterbach
Copy link
Copy Markdown
Member

Looks good - one question though: as with F2B, is there a special reason to not use the community repository all the time? The comfort of choosing whether to build with the Repo is very nice, I won't block merging, but it seems a bit unnecessary to me :)

@casperklein
Copy link
Copy Markdown
Member Author

There shouldn't be any problems for existing setups switching to the community repository, as the version gap is only minor. However to be safe, we can wait for the next DMS major release, to make the community repository the default.

It's just to avoid the minimal chance of issues fo existing setups. Therefore my proposal was, to make it now optional and later the default.

We could also switch right now. I am using the community repo for a long time and had never issues.

@georglauterbach
Copy link
Copy Markdown
Member

There shouldn't be any problems for existing setups switching to the community repository, as the version gap is only minor. However to be safe, we can wait for the next DMS major release, to make the community repository the default.

It's just to avoid the minimal chance of issues fo existing setups. Therefore my proposal was, to make it now optional and later the default.

We could also switch right now. I am using the community repo for a long time and had never issues.

I would go with postponing merging this until after the release of v11.2.0 and then just use the community repo for all builds :) But I am completely fine with your proposal too. What do other @docker-mailserver/maintainers say?

@casperklein
Copy link
Copy Markdown
Member Author

I would prefer merging this, as it has no drawbacks, but gives easily the ability to test the community version right now. Later it's pretty simple to remove the condition and make it the default.

@georglauterbach
Copy link
Copy Markdown
Member

I would prefer merging this, as it has no drawbacks, but gives easily the ability to test the community version right now. Later it's pretty simple to remove the condition and make it the default.

Very much fine with me :D

One last question: this PR includes changes from the currently open F2B PR right, so we merge the F2B PR beforehand?

@casperklein
Copy link
Copy Markdown
Member Author

Yeah, I've created a new branch based on the f2b branch. That's why f2b commits also show up here. f2b should be merged first.

@georglauterbach georglauterbach added the pr/waiting for other PR to get merged This PR is waiting for another / other PR(s) to get merged label Sep 26, 2022
@georglauterbach georglauterbach mentioned this pull request Sep 28, 2022
11 tasks
@georglauterbach georglauterbach removed the pr/waiting for other PR to get merged This PR is waiting for another / other PR(s) to get merged label Sep 29, 2022
@georglauterbach
Copy link
Copy Markdown
Member

Same old issue with CI: only 217 tests are executed because setup_file for tests.bats did not succeed. This become very annoying..

@wernerfred
Copy link
Copy Markdown
Member

wernerfred commented Sep 29, 2022

Shouldn't the changes be added to CHANGELOG.md like proposed/defined by @georglauterbach in #2789? Or am i missing something? Or does this count only from 11.2.0 onwards?

@georglauterbach
Copy link
Copy Markdown
Member

Shouldn't the changes be added to CHANGELOG.md like proposed/defined by @georglauterbach in #2789? Or am i missing something? Or does this count only from 11.2.0 onwards?

They should (in theory), but as the new changelog format has not been merged yet, I think it's fine for the follow-up PR that enabled the community Repo by default to add a changelog notice.

@casperklein
Copy link
Copy Markdown
Member Author

Same old issue with CI: only 217 tests are executed because setup_file for tests.bats did not succeed. This become very annoying..

On the 4th attempt, the tests run successful 🎉

@casperklein casperklein merged commit 157fde2 into docker-mailserver:master Sep 29, 2022
@casperklein casperklein deleted the dovecot branch September 29, 2022 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci 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.

4 participants