Open reneleonhardt opened 11 months ago
Thanks for looking into this. The way the changes are arranged makes it impossible to backport these to the currently maintained version 1.0.x. A lot of these changes make sense and are interesting though.
Thanks for looking into this. The way the changes are arranged makes it impossible to backport these to the currently maintained version 1.0.x. A lot of these changes make sense and are interesting though.
I can rebase onto main if that's what you mean by backport 😅
@mp911de Can you test this branch yourself and clarify what would be needed to merge updates?
Hi @reneleonhardt , I think what Mark wants is something like atomic commits, you need to rebase with main and send commits that are independent of each other, to give an example you need to separate the changes in individual commits like:
More or less essentially the list you provide in the description, but each should be an individual commit rebased on top of main.
BTW, normally when there are unrelated changes, the best thing is to send multiple PRs, but this also depends on the maintainer of the project if it's ok to have a single PR like this.
Unrelated changes?
There will be a myriad of conflicts when any other PR get's merged before all of them are merged into main, what is the advantage of 10 commits changing 1 line over 1 PR changing 10 lines in the same file?
Unrelated changes?
Updates
* Update Maven Wrapper to 3.3.2 and Maven to 3.9.8 * Add Dependabot configuration * Update maven plugins * Update dependencies * Build with JDK 17 for Java 8
Improve Tests
* Testcontainers only use JUnit Jupiter * Run tests with UTF-8 encoding and disable logging
There will be a myriad of conflicts when any other PR get's merged before all of them are merged into main, what is the advantage of 10 commits changing 1 line over 1 PR changing 10 lines in the same file?
Yes, unrelated changes, updating Maven Wrapper is not related and does not affect updating the dependabot configuration, and updating the tests is not related to building with JDK 17, and so on...
I'm not a maintainer here but is just a good practice to make commits atomic, what this means is that if you update one part of the code that updates dependencies, that's one commit, if you update SCRAM code (including tests) that's another commit, in general changes that can live "independent" of each other should be different commits, there is no global "Updates" or "Improve Tests" category.
What is the advantage of having 10 commits instead of a single one with all the changes mixed? Is simply maintainability, if one change (commit) turns out to be broken it can be reverted that single commit without affecting all others, another advantage is that if multiple branches need to be "supported" is easiest to cherry-pick one commit and apply it to that branch, maybe the Maven Wrapper update make sense to backport to an older branch, but maybe the SCRAM changes do not, so having them in a single commit makes it impossible to backport the changes to that branch.
Issue description
Additional context
There were no tests for PgPool, I just updated the Docker image.