From bf70ac842788f509d5610bad39ddd81ebc54d449 Mon Sep 17 00:00:00 2001 From: Dominik Zalewski Date: Fri, 20 Oct 2023 08:49:13 +0200 Subject: [PATCH 1/2] Reformat DEVELOPMENT.md: Git merge strategy to 72 characters column --- .github/DEVELOPMENT.md | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/.github/DEVELOPMENT.md b/.github/DEVELOPMENT.md index c1518da8c6c5..e0c66da7cd1d 100644 --- a/.github/DEVELOPMENT.md +++ b/.github/DEVELOPMENT.md @@ -26,22 +26,24 @@ A typical pull request should strive to contain a single logical change (but not necessarily a single commit). Unrelated changes should generally be extracted into their own PRs. -If a pull request does consist of multiple commits, it is expected that every -prefix of it is correct. That is, there might be preparatory commits at the -bottom of the stack that don't bring any value by themselves, but none of the -commits should introduce an error that is fixed by some future commit. Every -commit should build and pass all tests. - -Commit messages and history are also important, as they are used by other -developers to keep track of the motivation behind changes. Keep logical diffs -grouped together in separate commits, and order commits in a way that explains -the progress of the changes. Rewriting and reordering commits may be a necessary -part of the PR review process as the code changes. Mechanical changes (like -refactoring and renaming)should be separated from logical and functional -changes. E.g. deduplicating code or extracting helper methods should happen in a -separate commit from the commit where new features or behavior is introduced. -This makes reviewing the code much easier and reduces the chance of introducing -unintended changes in behavior. +If a pull request does consist of multiple commits, it is expected that +every prefix of it is correct. That is, there might be preparatory +commits at the bottom of the stack that don't bring any value by +themselves, but none of the commits should introduce an error that is +fixed by some future commit. Every commit should build and pass all +tests. + +Commit messages and history are also important, as they are used by +other developers to keep track of the motivation behind changes. Keep +logical diffs grouped together in separate commits, and order commits +in a way that explains the progress of the changes. Rewriting and +reordering commits may be a necessary part of the PR review process as +the code changes. Mechanical changes (like refactoring and +renaming)should be separated from logical and functional changes. E.g. +deduplicating code or extracting helper methods should happen in a +separate commit from the commit where new features or behavior is +introduced. This makes reviewing the code much easier and reduces the +chance of introducing unintended changes in behavior. ## Code Style From 59dbe44e0caff953b5b6ee4d184652f46aa66fb6 Mon Sep 17 00:00:00 2001 From: Dominik Zalewski Date: Wed, 11 Oct 2023 14:57:26 +0200 Subject: [PATCH 2/2] Update DEVELOPMENT.md: Git merge strategy --- .github/DEVELOPMENT.md | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/DEVELOPMENT.md b/.github/DEVELOPMENT.md index e0c66da7cd1d..3da05a0f6b41 100644 --- a/.github/DEVELOPMENT.md +++ b/.github/DEVELOPMENT.md @@ -26,24 +26,26 @@ A typical pull request should strive to contain a single logical change (but not necessarily a single commit). Unrelated changes should generally be extracted into their own PRs. -If a pull request does consist of multiple commits, it is expected that -every prefix of it is correct. That is, there might be preparatory -commits at the bottom of the stack that don't bring any value by -themselves, but none of the commits should introduce an error that is -fixed by some future commit. Every commit should build and pass all -tests. - -Commit messages and history are also important, as they are used by -other developers to keep track of the motivation behind changes. Keep -logical diffs grouped together in separate commits, and order commits -in a way that explains the progress of the changes. Rewriting and -reordering commits may be a necessary part of the PR review process as -the code changes. Mechanical changes (like refactoring and -renaming)should be separated from logical and functional changes. E.g. -deduplicating code or extracting helper methods should happen in a -separate commit from the commit where new features or behavior is -introduced. This makes reviewing the code much easier and reduces the -chance of introducing unintended changes in behavior. +If a pull request contains a stack of more than one commit, then +popping any number of commits from the top of the stack, should not +break the PR, ie. every commit should build and pass all tests. + +Commit messages and history are important as well, because they are +used by other developers to keep track of the motivation behind +changes. Keep logical diffs grouped together in separate commits and +order commits in a way that explains by itself the evolution of the +change. Rewriting and reordering commits is a natural part of the +review process. Mechanical changes like refactoring, renaming, removing +duplication, extracting helper methods, static imports should be kept +separated from logical and functional changes like adding a new feature +or modifying code behaviour. This makes reviewing the code much easier +and reduces the chance of introducing unintended changes in behavior. + +Whenever in doubt on splitting a change into a separate commit, ask +yourself the following question: if all other work in the PR needs to +be reverted after merging to master for some objective reason (eg. a +bug has been discovered), is it worth keeping that commit still in +master. ## Code Style