git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] SubmittingPatchs: clarify choice of base and testing

From
Fabian Stelzer <fs@gigacodes.de>
Date
Dec 30, 2021, 10:20 UTC
Message-ID
<20211230102050.oe2t5pavijaow5lx@fs>
In-Reply-To
<xmqq5yre7w5o.fsf@gitster.g>
On 23.12.2021 15:12, Junio C Hamano wrote:
Just some minor pointers/typos. Looks good.
Show 62 quoted lines
>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 <gitster@pobox.com>
>---
> 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.
>
>diff --git c/Documentation/SubmittingPatches w/Documentation/SubmittingPatches
>index e409022d93..3c4c5d9f18 100644
>--- c/Documentation/SubmittingPatches
>+++ w/Documentation/SubmittingPatches
>@@ -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
Show 21 quoted lines
>+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?

Show 22 quoted lines
>+
>+. 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.
>+
Maybe add a hint to `--base=` for format-patch?
Show 15 quoted lines
>
> [[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]
Previous: Elijah NewrenNext: Junio C Hamano
Message 46 of 63 in “midx: clean up t5319 under 'SANITIZE=leak'”
  1. 00/11 midx: clean up t5319 under 'SANITIZE=leak'Taylor Blau, Oct 21, 2021
  2. 01/11 midx.c: clean up chunkfile after reading the MIDXTaylor Blau, Oct 21, 2021
  3. Junio C HamanoOct 21, 2021
  4. Ævar Arnfjörð BjarmasonOct 21, 2021
  5. Junio C HamanoOct 21, 2021
  6. Taylor BlauOct 22, 2021
  7. 02/11 midx.c: don't leak MIDX from verify_midx_fileTaylor Blau, Oct 21, 2021
  8. Eric SunshineOct 21, 2021
  9. Junio C HamanoOct 21, 2021
  10. Junio C HamanoOct 21, 2021
  11. 03/11 t/helper/test-read-midx.c: free MIDX within read_midx_file()Taylor Blau, Oct 21, 2021
  12. 04/11 builtin/pack-objects.c: don't leak memory via argumentsTaylor Blau, Oct 21, 2021
  13. 05/11 builtin/repack.c: avoid leaking child argumentsTaylor Blau, Oct 21, 2021
  14. Derrick StoleeOct 21, 2021
  15. Junio C HamanoOct 21, 2021
  16. Junio C HamanoOct 21, 2021
  17. Taylor BlauOct 22, 2021
  18. 06/11 builtin/multi-pack-index.c: don't leak concatenated optionsTaylor Blau, Oct 21, 2021
  19. 07/11 pack-bitmap.c: avoid leaking via midx_bitmap_filename()Taylor Blau, Oct 21, 2021
  20. Junio C HamanoOct 21, 2021
  21. Taylor BlauOct 22, 2021
  22. 08/11 pack-bitmap.c: don't leak type-level bitmapsTaylor Blau, Oct 21, 2021
  23. Junio C HamanoOct 21, 2021
  24. 09/11 pack-bitmap.c: more aggressively free in free_bitmap_index()Taylor Blau, Oct 21, 2021
  25. Eric SunshineOct 21, 2021
  26. Junio C HamanoOct 21, 2021
  27. Taylor BlauOct 22, 2021
  28. Junio C HamanoOct 21, 2021
  29. 11/11 t5319: UNLEAK() the remaining leaksTaylor Blau, Oct 21, 2021
  30. 10/11 pack-bitmap-write.c: don't return without stop_progress()Taylor Blau, Oct 21, 2021
  31. Eric SunshineOct 21, 2021
  32. Ævar Arnfjörð BjarmasonOct 21, 2021
  33. Junio C HamanoOct 21, 2021
  34. Taylor BlauOct 22, 2021
  35. Junio C HamanoOct 23, 2021
  36. SubmittingPatchs: clarify choice of base and testingJunio C Hamano, Oct 23, 2021
  37. Ævar Arnfjörð BjarmasonOct 23, 2021
  38. Junio C HamanoOct 23, 2021
  39. Junio C HamanoOct 23, 2021
  40. Fabian StelzerOct 25, 2021
  41. Junio C HamanoOct 25, 2021
  42. Junio C HamanoOct 25, 2021
  43. Junio C HamanoOct 25, 2021
  44. SubmittingPatchs: clarify choice of base and testingJunio C Hamano, Dec 23, 2021
  45. Elijah NewrenDec 28, 2021
  46. Fabian StelzerDec 30, 2021
  47. Re* [PATCH v2] SubmittingPatchs: clarify choice of base and testingJunio C Hamano, Dec 30, 2021
  48. Ævar Arnfjörð BjarmasonOct 21, 2021
  49. Taylor BlauOct 22, 2021
  50. Ævar Arnfjörð BjarmasonOct 22, 2021
  51. leak tests: add an interface to the LSAN_OPTIONS "suppressions"Ævar Arnfjörð Bjarmason, Oct 22, 2021
  52. Taylor BlauOct 26, 2021
  53. Jeff KingOct 26, 2021
  54. Taylor BlauOct 26, 2021
  55. Jeff KingOct 26, 2021
  56. Ævar Arnfjörð BjarmasonOct 27, 2021
  57. Jeff KingOct 27, 2021
  58. Junio C HamanoOct 27, 2021
  59. Ævar Arnfjörð BjarmasonOct 27, 2021
  60. Jeff KingOct 29, 2021
  61. Jeff KingOct 29, 2021
  62. Ævar Arnfjörð BjarmasonOct 27, 2021
  63. Derrick StoleeOct 21, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.