From 5e8f611d5389e0dde8de6603f780ba365096b14e Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Wed, 22 Dec 2021 18:16:32 -0300 Subject: [PATCH 1/6] doc: additional guidance about amending commits In my first contribution, I got the amending guidance wrongly and amended my commit to attend some requested changes. So I added some more lines in the docs to help the further contributors to don't make the same mistake that I did --- doc/guides/contributing/pull-requests.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index 228b9839ca1f8b..1ad39ef8472b18 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -318,6 +318,8 @@ $ git commit --amend $ git push --force-with-lease origin my-branch ``` +Amend your last commit just to fix some minor mistake you may have noticed before any reviewing, though. To make the reviewing process simpler, when some changes are requested it's preferable to add a new commit. + There are a number of more advanced mechanisms for managing commits using `git rebase` that can be used, but are beyond the scope of this guide. From 758e7ab7ab3ad20f2ab0ff09ca1f34c8386775f9 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Thu, 23 Dec 2021 13:19:42 -0300 Subject: [PATCH 2/6] doc: removing commit --amend recommendation --- doc/guides/contributing/pull-requests.md | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index 1ad39ef8472b18..921524b23e12b5 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -309,17 +309,6 @@ $ git push --force-with-lease origin my-branch to delete history in `git`. Before you use it, make sure you understand the risks. If in doubt, you can always ask for guidance in the pull request. -If you happen to make a mistake in any of your commits, do not worry. You can -amend the last commit (for example if you want to change the commit log). - -```text -$ git add any/changed/files -$ git commit --amend -$ git push --force-with-lease origin my-branch -``` - -Amend your last commit just to fix some minor mistake you may have noticed before any reviewing, though. To make the reviewing process simpler, when some changes are requested it's preferable to add a new commit. - There are a number of more advanced mechanisms for managing commits using `git rebase` that can be used, but are beyond the scope of this guide. From 712175f2602b09566373eb80e67eaf7879576d95 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Thu, 23 Dec 2021 15:24:39 -0300 Subject: [PATCH 3/6] doc: adding more insight about force pushing --- doc/guides/contributing/pull-requests.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index 921524b23e12b5..f1e5e8f8dbbc89 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -308,6 +308,8 @@ $ git push --force-with-lease origin my-branch **Important:** The `git push --force-with-lease` command is one of the few ways to delete history in `git`. Before you use it, make sure you understand the risks. If in doubt, you can always ask for guidance in the pull request. +Force-pushing also complicates the review process, as it won't allow reviewers +to get a quick glance on what have changed. There are a number of more advanced mechanisms for managing commits using `git rebase` that can be used, but are beyond the scope of this guide. From 5695a242661219a73575ef8f2ba6438505632515 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Thu, 23 Dec 2021 16:08:17 -0300 Subject: [PATCH 4/6] Update doc/guides/contributing/pull-requests.md Making all drawbacks to be listed in a row Co-authored-by: Antoine du Hamel --- doc/guides/contributing/pull-requests.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index f1e5e8f8dbbc89..43cf9cfb0e21da 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -306,8 +306,10 @@ $ git push --force-with-lease origin my-branch ``` **Important:** The `git push --force-with-lease` command is one of the few ways -to delete history in `git`. Before you use it, make sure you understand the -risks. If in doubt, you can always ask for guidance in the pull request. +to delete history in `git`. It also complicates the review process, as it won't +allow reviewers to get a quick glance on what changed. Before you use it, make +sure you understand the risks. If in doubt, you can always ask for guidance in +the pull request. Force-pushing also complicates the review process, as it won't allow reviewers to get a quick glance on what have changed. From 3559e447f4c70546950ec860f5b2f9738cf880c4 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Thu, 23 Dec 2021 16:11:20 -0300 Subject: [PATCH 5/6] doc: putting the new tips before 'Before you use it' This way all the drawbacks are in a row --- doc/guides/contributing/pull-requests.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index 43cf9cfb0e21da..f8ed15d580525c 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -307,11 +307,11 @@ $ git push --force-with-lease origin my-branch **Important:** The `git push --force-with-lease` command is one of the few ways to delete history in `git`. It also complicates the review process, as it won't -allow reviewers to get a quick glance on what changed. Before you use it, make +allow reviewers to get a quick glance on what changed. +Force-pushing also complicates the review process, as it won't allow reviewers +to get a quick glance on what have changed. Before you use it, make sure you understand the risks. If in doubt, you can always ask for guidance in the pull request. -Force-pushing also complicates the review process, as it won't allow reviewers -to get a quick glance on what have changed. There are a number of more advanced mechanisms for managing commits using `git rebase` that can be used, but are beyond the scope of this guide. From 98d15ba40964243d7fcb9e01425797d119aef1cc Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Thu, 23 Dec 2021 16:34:58 -0300 Subject: [PATCH 6/6] Update doc/guides/contributing/pull-requests.md Fixing duplicated additional guidance about for pushing Co-authored-by: Antoine du Hamel --- doc/guides/contributing/pull-requests.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index f8ed15d580525c..cd2d6dcdc39c1a 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -307,9 +307,7 @@ $ git push --force-with-lease origin my-branch **Important:** The `git push --force-with-lease` command is one of the few ways to delete history in `git`. It also complicates the review process, as it won't -allow reviewers to get a quick glance on what changed. -Force-pushing also complicates the review process, as it won't allow reviewers -to get a quick glance on what have changed. Before you use it, make +allow reviewers to get a quick glance on what changed. Before you use it, make sure you understand the risks. If in doubt, you can always ask for guidance in the pull request.