From: Junio C Hamano <hidden> Date: 2021-10-23 20:32:38
We encourage identifying what, among many topics on `next`, exact
topics a new work depends on, instead of building directly on
`next`. Let's clarify this in the documentation.
Developers should know what they are building on top of, and be
aware of which part of the system is currently being worked on.
Encouraging them to make trial merges to `next` and `seen`
themselves will incentivize them to read others' changes and
understand them, eventually helping the developers to coordinate
among themselves and reviewing each others' changes.
Signed-off-by: Junio C Hamano <redacted>
---
* This, being a guide for those who submit their work, stops short
of telling them to send reviews on others' patch that interacts
with their work, but reading and understanding others' work is
both necessary to make their own work to play well together with
other topics, and to start reviewing others' work.
Documentation/SubmittingPatches | 50 ++++++++++++++++++++++++++++++-----------
1 file changed, 37 insertions(+), 13 deletions(-)
@@ -19,8 +19,11 @@ change is relevant to. base your work on the tip of the topic. * A new feature should be based on `master` in general. If the new- feature depends on a topic that is in `seen`, but not in `master`,- base your work on the tip of that topic.+ feature depends on other topics that are in `next`, but not in+ `master`, fork a branch from the tip of `master`, merge these topics+ to the branch, and work on that branch. You can remind yourself of+ how you prepared the base with `git log --first-parent master..`+ easily by doing so. * Corrections and enhancements to a topic not yet in `master` should be based on the tip of that topic. If the topic has not been merged
@@ -28,10 +31,10 @@ change is relevant to. into the series. * In the exceptional case that a new feature depends on several topics- not in `master`, start working on `next` or `seen` privately and send- out patches for discussion. Before the final merge, you may have to- wait until some of the dependent topics graduate to `master`, and- rebase your work.+ not in `master`, start working on `next` or `seen` privately and+ send out patches only for discussion. Once your new feature starts+ to stabilize, you would have to rebase it (see the "depends on other+ topics" above). * Some parts of the system have dedicated maintainers with their own repositories (see the section "Subsystems" below). Changes to
@@ -71,8 +74,13 @@ Make sure that you have tests for the bug you are fixing. See [[tests]] When adding a new feature, make sure that you have new tests to show the feature triggers the new behavior when it should, and to show the-feature does not trigger when it shouldn't. After any code change, make-sure that the entire test suite passes.+feature does not trigger when it shouldn't. After any code change,+make sure that the entire test suite passes. When fixing a bug, make+sure you have new tests that breaks if somebody else breaks what you+fixed by accident to avoid regression. Also, try merging your work to+'next' and 'seen' and make sure the tests still pass; topics by others+that are still in flight may have unexpected interactions with what+you are trying to do in your topic. Pushing to a fork of https://github.com/git/git will use their CI integration to test your changes on Linux, Mac and Windows. See the
@@ -144,8 +152,21 @@ without external resources. Instead of giving a URL to a mailing list archive, summarize the relevant points of the discussion. [[commit-reference]]-If you want to reference a previous commit in the history of a stable-branch, use the format "abbreviated hash (subject, date)", like this:++There are a few reasons why you may want to refer to another commit in+the "more stable" part of the history (i.e. on branches like `maint`,+`master`, and `next`):++. A commit that introduced the root cause of a bug you are fixing.++. A commit that introduced a feature that is what you are enhancing.++. A commit that conflicts with your work when you made a trial merge+ of your work into `next` and `seen` for testing.++When you reference a commit on a more stable branch (like `master`,+`maint` and `next`), use the format "abbreviated hash (subject,+date)", like this: .... Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
@@ -260,8 +281,8 @@ or include any extra files which do not relate to what your patch is trying to achieve. Make sure to review your patch after generating it, to ensure accuracy. Before sending out, please make sure it cleanly applies to the `master`-branch head. If you are preparing a work based on "next" branch,-that is fine, but please mark it as such.+branch head. If you are preparing a work based on selected topics+merged to `master`, please mark your patch as such. [[send-patches]] === Sending your patches.
@@ -365,7 +386,10 @@ Security mailing list{security-ml-ref}. Send your patch with "To:" set to the mailing list, with "cc:" listing people who are involved in the area you are touching (the `git contacts` command in `contrib/contacts/` can help to-identify them), to solicit comments and reviews.+identify them), to solicit comments and reviews. Also, when you made+trial merges of your topic to `next` and `seen`, you may have noticed+work by others conflicting with your changes. There is a good possibility+that these people may know the area you are touching well. :current-maintainer: footnote:[The current maintainer: gitster@pobox.com] :git-ml: footnote:[The mailing list: git@vger.kernel.org]
@@ -19,8 +19,11 @@ change is relevant to. base your work on the tip of the topic. * A new feature should be based on `master` in general. If the new- feature depends on a topic that is in `seen`, but not in `master`,- base your work on the tip of that topic.+ feature depends on other topics that are in `next`, but not in+ `master`, fork a branch from the tip of `master`, merge these topics+ to the branch, and work on that branch. You can remind yourself of+ how you prepared the base with `git log --first-parent master..`+ easily by doing so.
For what it's worth when I do this I base things on one either your
gitster/* branches, what I've applied from the ML, or grabbed from the
author's own repo, and then usually rebase it on the latest "master" or
<topic> as appropriate so I don't have on old base, particularly in the
case of gitster/*, you don't rebase those unless needed.
Anyway, I can see why you suggest the "base on master and merge", it has
its benefits, but it is in apparent conflict with existing advice added
in 76644e3268b (documentation: add tutorial for first contribution,
2019-05-17).
I.e. the "After Review Approval" section (of all places) discusses what
to do in that situation. It is narrowly talking about submitting
something on top of your own topic, but the advice should be the same in
either case I'd think.
quoted hunk
* Corrections and enhancements to a topic not yet in `master` should
be based on the tip of that topic. If the topic has not been merged
@@ -28,10 +31,10 @@ change is relevant to. into the series. * In the exceptional case that a new feature depends on several topics- not in `master`, start working on `next` or `seen` privately and send- out patches for discussion. Before the final merge, you may have to- wait until some of the dependent topics graduate to `master`, and- rebase your work.+ not in `master`, start working on `next` or `seen` privately and+ send out patches only for discussion. Once your new feature starts+ to stabilize, you would have to rebase it (see the "depends on other+ topics" above). * Some parts of the system have dedicated maintainers with their own repositories (see the section "Subsystems" below). Changes to
@@ -71,8 +74,13 @@ Make sure that you have tests for the bug you are fixing. See [[tests]] When adding a new feature, make sure that you have new tests to show the feature triggers the new behavior when it should, and to show the-feature does not trigger when it shouldn't. After any code change, make-sure that the entire test suite passes.+feature does not trigger when it shouldn't. After any code change,+make sure that the entire test suite passes. When fixing a bug, make+sure you have new tests that breaks if somebody else breaks what you+fixed by accident to avoid regression. Also, try merging your work to+'next' and 'seen' and make sure the tests still pass; topics by others+that are still in flight may have unexpected interactions with what+you are trying to do in your topic.
Maybe it would be useful to have a CI target that merged to next/master
and reported how that went? It would have to be a soft failure, as we
might have an easy merge conflict, or someone's pushing a revision to a
topic that's already in "next" or "seen" (and might conflict). But
having it as an FYI might be helpful.
quoted hunk
Pushing to a fork of https://github.com/git/git will use their CI
integration to test your changes on Linux, Mac and Windows. See the
@@ -144,8 +152,21 @@ without external resources. Instead of giving a URL to a mailing list archive, summarize the relevant points of the discussion. [[commit-reference]]-If you want to reference a previous commit in the history of a stable-branch, use the format "abbreviated hash (subject, date)", like this:++There are a few reasons why you may want to refer to another commit in+the "more stable" part of the history (i.e. on branches like `maint`,+`master`, and `next`):++. A commit that introduced the root cause of a bug you are fixing.++. A commit that introduced a feature that is what you are enhancing.++. A commit that conflicts with your work when you made a trial merge+ of your work into `next` and `seen` for testing.++When you reference a commit on a more stable branch (like `master`,+`maint` and `next`), use the format "abbreviated hash (subject,+date)", like this:
This seems like a good clarification, but partially unrelated to the
$subject, i.e. just the last bullet point is directly relevant, the
first two are new general advice. Perhaps split those out into another
commit?
On 23.10.21 22:32, Junio C Hamano wrote:> * A new feature should be
based on `master` in general. If the new
- feature depends on a topic that is in `seen`, but not in `master`,
- base your work on the tip of that topic.
+ feature depends on other topics that are in `next`, but not in
+ `master`, fork a branch from the tip of `master`, merge these topics
+ to the branch, and work on that branch. You can remind yourself of
+ how you prepared the base with `git log --first-parent master..`
+ easily by doing so.
Using the topic branches from gitster/git that were merged? Or by
selecting the specific commits from the merge into next?
quoted hunk
@@ -260,8 +281,8 @@ or include any extra files which do not relate to what your patch is trying to achieve. Make sure to review your patch after generating it, to ensure accuracy. Before sending out, please make sure it cleanly applies to the `master`-branch head. If you are preparing a work based on "next" branch,-that is fine, but please mark it as such.+branch head. If you are preparing a work based on selected topics+merged to `master`, please mark your patch as such.
I think this meant to say 'merged to "next|maint|seen"'?
Or topics selected for being merged into master?
Thanks,
Fabian
From: Junio C Hamano <hidden> Date: 2021-12-23 23:12:56
We encourage identifying what, among many topics on `next`, exact
topics a new work depends on, instead of building directly on
`next`. Let's clarify this in the documentation.
Developers should know what they are building on top of, and be
aware of which part of the system is currently being worked on.
Encouraging them to make trial merges to `next` and `seen`
themselves will incentivize them to read others' changes and
understand them, eventually helping the developers to coordinate
among themselves and reviewing each others' changes.
Signed-off-by: Junio C Hamano <redacted>
---
Documentation/SubmittingPatches | 53 ++++++++++++++++++++++++++++++-----------
1 file changed, 39 insertions(+), 14 deletions(-)
* I've been trying to clear the deck, and noticed that this has
been untended for quite some time. With some clarification to
a place I was even confused myself while responding to Fabian's
comments in the earlier round.
@@ -19,8 +19,10 @@ change is relevant to. base your work on the tip of the topic. * A new feature should be based on `master` in general. If the new- feature depends on a topic that is in `seen`, but not in `master`,- base your work on the tip of that topic.+ feature depends on other topics that are in `next`, but not in+ `master`, fork a branch from the tip of `master`, merge these topics+ to the branch, and work on that branch. You can remind yourself of+ how you prepared the base with `git log --first-parent master..`. * Corrections and enhancements to a topic not yet in `master` should be based on the tip of that topic. If the topic has not been merged
@@ -28,10 +30,10 @@ change is relevant to. into the series. * In the exceptional case that a new feature depends on several topics- not in `master`, start working on `next` or `seen` privately and send- out patches for discussion. Before the final merge, you may have to- wait until some of the dependent topics graduate to `master`, and- rebase your work.+ not in `master`, start working on `next` or `seen` privately and+ send out patches only for discussion. Once your new feature starts+ to stabilize, you would have to rebase it (see the "depends on other+ topics" above). * Some parts of the system have dedicated maintainers with their own repositories (see the section "Subsystems" below). Changes to
@@ -71,8 +73,13 @@ Make sure that you have tests for the bug you are fixing. See [[tests]] When adding a new feature, make sure that you have new tests to show the feature triggers the new behavior when it should, and to show the-feature does not trigger when it shouldn't. After any code change, make-sure that the entire test suite passes.+feature does not trigger when it shouldn't. After any code change,+make sure that the entire test suite passes. When fixing a bug, make+sure you have new tests that breaks if somebody else breaks what you+fixed by accident to avoid regression. Also, try merging your work to+'next' and 'seen' and make sure the tests still pass; topics by others+that are still in flight may have unexpected interactions with what+you are trying to do in your topic. Pushing to a fork of https://github.com/git/git will use their CI integration to test your changes on Linux, Mac and Windows. See the
@@ -144,8 +151,21 @@ without external resources. Instead of giving a URL to a mailing list archive, summarize the relevant points of the discussion. [[commit-reference]]-If you want to reference a previous commit in the history of a stable-branch, use the format "abbreviated hash (subject, date)", like this:++There are a few reasons why you may want to refer to another commit in+the "more stable" part of the history (i.e. on branches like `maint`,+`master`, and `next`):++. A commit that introduced the root cause of a bug you are fixing.++. A commit that introduced a feature that is what you are enhancing.++. A commit that conflicts with your work when you made a trial merge+ of your work into `next` and `seen` for testing.++When you reference a commit on a more stable branch (like `master`,+`maint` and `next`), use the format "abbreviated hash (subject,+date)", like this: .... Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
@@ -259,9 +279,11 @@ Please make sure your patch does not add commented out debugging code, or include any extra files which do not relate to what your patch is trying to achieve. Make sure to review your patch after generating it, to ensure accuracy. Before-sending out, please make sure it cleanly applies to the `master`-branch head. If you are preparing a work based on "next" branch,-that is fine, but please mark it as such.+sending out, please make sure it cleanly applies to the base you+have chosen in the "Decide what to base your work on" section,+and unless it targets the `master` branch (which is the default),+mark your patches as such.+ [[send-patches]] === Sending your patches.
@@ -365,7 +387,10 @@ Security mailing list{security-ml-ref}. Send your patch with "To:" set to the mailing list, with "cc:" listing people who are involved in the area you are touching (the `git contacts` command in `contrib/contacts/` can help to-identify them), to solicit comments and reviews.+identify them), to solicit comments and reviews. Also, when you made+trial merges of your topic to `next` and `seen`, you may have noticed+work by others conflicting with your changes. There is a good possibility+that these people may know the area you are touching well. :current-maintainer: footnote:[The current maintainer: gitster@pobox.com] :git-ml: footnote:[The mailing list: git@vger.kernel.org]
On Fri, Dec 24, 2021 at 5:25 PM Junio C Hamano [off-list ref] wrote:
quoted hunk
We encourage identifying what, among many topics on `next`, exact
topics a new work depends on, instead of building directly on
`next`. Let's clarify this in the documentation.
Developers should know what they are building on top of, and be
aware of which part of the system is currently being worked on.
Encouraging them to make trial merges to `next` and `seen`
themselves will incentivize them to read others' changes and
understand them, eventually helping the developers to coordinate
among themselves and reviewing each others' changes.
Signed-off-by: Junio C Hamano <redacted>
---
Documentation/SubmittingPatches | 53 ++++++++++++++++++++++++++++++-----------
1 file changed, 39 insertions(+), 14 deletions(-)
* I've been trying to clear the deck, and noticed that this has
been untended for quite some time. With some clarification to
a place I was even confused myself while responding to Fabian's
comments in the earlier round.
@@ -19,8 +19,10 @@ change is relevant to. base your work on the tip of the topic. * A new feature should be based on `master` in general. If the new- feature depends on a topic that is in `seen`, but not in `master`,- base your work on the tip of that topic.+ feature depends on other topics that are in `next`, but not in+ `master`, fork a branch from the tip of `master`, merge these topics+ to the branch, and work on that branch. You can remind yourself of+ how you prepared the base with `git log --first-parent master..`. * Corrections and enhancements to a topic not yet in `master` should be based on the tip of that topic. If the topic has not been merged
@@ -28,10 +30,10 @@ change is relevant to. into the series. * In the exceptional case that a new feature depends on several topics- not in `master`, start working on `next` or `seen` privately and send- out patches for discussion. Before the final merge, you may have to- wait until some of the dependent topics graduate to `master`, and- rebase your work.+ not in `master`, start working on `next` or `seen` privately and+ send out patches only for discussion. Once your new feature starts+ to stabilize, you would have to rebase it (see the "depends on other+ topics" above). * Some parts of the system have dedicated maintainers with their own repositories (see the section "Subsystems" below). Changes to
@@ -71,8 +73,13 @@ Make sure that you have tests for the bug you are fixing. See [[tests]] When adding a new feature, make sure that you have new tests to show the feature triggers the new behavior when it should, and to show the-feature does not trigger when it shouldn't. After any code change, make-sure that the entire test suite passes.+feature does not trigger when it shouldn't. After any code change,+make sure that the entire test suite passes. When fixing a bug, make+sure you have new tests that breaks if somebody else breaks what you+fixed by accident to avoid regression. Also, try merging your work to+'next' and 'seen' and make sure the tests still pass; topics by others+that are still in flight may have unexpected interactions with what+you are trying to do in your topic. Pushing to a fork of https://github.com/git/git will use their CI integration to test your changes on Linux, Mac and Windows. See the
@@ -144,8 +151,21 @@ without external resources. Instead of giving a URL to a mailing list archive, summarize the relevant points of the discussion. [[commit-reference]]-If you want to reference a previous commit in the history of a stable-branch, use the format "abbreviated hash (subject, date)", like this:++There are a few reasons why you may want to refer to another commit in+the "more stable" part of the history (i.e. on branches like `maint`,+`master`, and `next`):++. A commit that introduced the root cause of a bug you are fixing.++. A commit that introduced a feature that is what you are enhancing.++. A commit that conflicts with your work when you made a trial merge+ of your work into `next` and `seen` for testing.++When you reference a commit on a more stable branch (like `master`,+`maint` and `next`), use the format "abbreviated hash (subject,+date)", like this:
I was going to comment that this would be a good place to mention
--pretty=reference, but looking at the file in question, that is
exactly what the text after this already does.
quoted hunk
....
Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
@@ -259,9 +279,11 @@ Please make sure your patch does not add commented out debugging code, or include any extra files which do not relate to what your patch is trying to achieve. Make sure to review your patch after generating it, to ensure accuracy. Before-sending out, please make sure it cleanly applies to the `master`-branch head. If you are preparing a work based on "next" branch,-that is fine, but please mark it as such.+sending out, please make sure it cleanly applies to the base you+have chosen in the "Decide what to base your work on" section,+and unless it targets the `master` branch (which is the default),+mark your patches as such.+ [[send-patches]] === Sending your patches.
@@ -365,7 +387,10 @@ Security mailing list{security-ml-ref}. Send your patch with "To:" set to the mailing list, with "cc:" listing people who are involved in the area you are touching (the `git contacts` command in `contrib/contacts/` can help to-identify them), to solicit comments and reviews.+identify them), to solicit comments and reviews. Also, when you made+trial merges of your topic to `next` and `seen`, you may have noticed+work by others conflicting with your changes. There is a good possibility+that these people may know the area you are touching well. :current-maintainer: footnote:[The current maintainer: gitster@pobox.com] :git-ml: footnote:[The mailing list: git@vger.kernel.org]
On 23.12.2021 15:12, Junio C Hamano wrote:
Just some minor pointers/typos. Looks good.
quoted hunk
We encourage identifying what, among many topics on `next`, exact
topics a new work depends on, instead of building directly on
`next`. Let's clarify this in the documentation.
Developers should know what they are building on top of, and be
aware of which part of the system is currently being worked on.
Encouraging them to make trial merges to `next` and `seen`
themselves will incentivize them to read others' changes and
understand them, eventually helping the developers to coordinate
among themselves and reviewing each others' changes.
Signed-off-by: Junio C Hamano <redacted>
---
Documentation/SubmittingPatches | 53 ++++++++++++++++++++++++++++++-----------
1 file changed, 39 insertions(+), 14 deletions(-)
* I've been trying to clear the deck, and noticed that this has
been untended for quite some time. With some clarification to
a place I was even confused myself while responding to Fabian's
comments in the earlier round.
@@ -19,8 +19,10 @@ change is relevant to. base your work on the tip of the topic.
* A new feature should be based on `master` in general. If the new
- feature depends on a topic that is in `seen`, but not in `master`,
- base your work on the tip of that topic.
+ feature depends on other topics that are in `next`, but not in
+ `master`, fork a branch from the tip of `master`, merge these topics
+ to the branch, and work on that branch. You can remind yourself of
+ how you prepared the base with `git log --first-parent master..`.
* Corrections and enhancements to a topic not yet in `master` should
be based on the tip of that topic. If the topic has not been merged
@@ -28,10 +30,10 @@ change is relevant to. into the series.
* In the exceptional case that a new feature depends on several topics
- not in `master`, start working on `next` or `seen` privately and send
- out patches for discussion. Before the final merge, you may have to
- wait until some of the dependent topics graduate to `master`, and
- rebase your work.
+ not in `master`, start working on `next` or `seen` privately and
+ send out patches only for discussion. Once your new feature starts
+ to stabilize, you would have to rebase it (see the "depends on other
+ topics" above).
* Some parts of the system have dedicated maintainers with their own
repositories (see the section "Subsystems" below). Changes to
@@ -71,8 +73,13 @@ Make sure that you have tests for the bug you are fixing. See
[[tests]]
When adding a new feature, make sure that you have new tests to show
the feature triggers the new behavior when it should, and to show the
-feature does not trigger when it shouldn't. After any code change, make
-sure that the entire test suite passes.
+feature does not trigger when it shouldn't. After any code change,
+make sure that the entire test suite passes. When fixing a bug, make
+sure you have new tests that breaks if somebody else breaks what you
s/breaks/break
quoted hunk
+fixed by accident to avoid regression. Also, try merging your work to
+'next' and 'seen' and make sure the tests still pass; topics by others
+that are still in flight may have unexpected interactions with what
+you are trying to do in your topic.
Pushing to a fork of https://github.com/git/git will use their CI
integration to test your changes on Linux, Mac and Windows. See the
@@ -144,8 +151,21 @@ without external resources. Instead of giving a URL to a mailing list
archive, summarize the relevant points of the discussion.
[[commit-reference]]
-If you want to reference a previous commit in the history of a stable
-branch, use the format "abbreviated hash (subject, date)", like this:
+
+There are a few reasons why you may want to refer to another commit in
+the "more stable" part of the history (i.e. on branches like `maint`,
+`master`, and `next`):
+
+. A commit that introduced the root cause of a bug you are fixing.
+
+. A commit that introduced a feature that is what you are enhancing.
I'm not a native speaker, but `that is what` sounds wrong to me.
`feature which you are enhancing` or `feature that you are enhancing` maybe?
quoted hunk
+
+. A commit that conflicts with your work when you made a trial merge
+ of your work into `next` and `seen` for testing.
+
+When you reference a commit on a more stable branch (like `master`,
+`maint` and `next`), use the format "abbreviated hash (subject,
+date)", like this:
....
Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
@@ -259,9 +279,11 @@ Please make sure your patch does not add commented out debugging code,
or include any extra files which do not relate to what your patch
is trying to achieve. Make sure to review
your patch after generating it, to ensure accuracy. Before
-sending out, please make sure it cleanly applies to the `master`
-branch head. If you are preparing a work based on "next" branch,
-that is fine, but please mark it as such.
+sending out, please make sure it cleanly applies to the base you
+have chosen in the "Decide what to base your work on" section,
+and unless it targets the `master` branch (which is the default),
+mark your patches as such.
+
Send your patch with "To:" set to the mailing list, with "cc:" listing
people who are involved in the area you are touching (the `git
contacts` command in `contrib/contacts/` can help to
-identify them), to solicit comments and reviews.
+identify them), to solicit comments and reviews. Also, when you made
+trial merges of your topic to `next` and `seen`, you may have noticed
+work by others conflicting with your changes. There is a good possibility
+that these people may know the area you are touching well.
:current-maintainer: footnote:[The current maintainer: gitster@pobox.com]
:git-ml: footnote:[The mailing list: git@vger.kernel.org]