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

Re: [PATCH v2 2/3] submodule: port submodule subcommand 'add' from shell to C

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Nov 18, 2020, 23:13 UTC
Message-ID
<20201118231331.716110-1-jonathantanmy@google.com>
In-Reply-To
<xmqqd01sugrg.fsf@gitster.c.googlers.com>
Show 8 quoted lines
> Whew.
> 
> This was way too big to be reviewed in a single sitting.  I do not
> know offhand if there is a better way to structure the changes into
> a more digestible pieces to help prevent reviewers from overlooking
> potential mistakes, though.
> 
> Thanks.

I just took a look at this, and one thing that would have helped is if you ported the end of the function first in a commit, and work your way backwards (in one or more commits).

After reading through the whole thing, I saw that this is mostly a straightforward start-to-finish port (besides factoring out code into functions), but it would be much easier for reviewers to conceptualize and discuss the different parts if they were already divided.

Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 11 of 16 in “submodule: port subcommand add from shell to C”
  1. 0/3 submodule: port subcommand add from shell to CShourya Shukla, Oct 7, 2020
  2. 1/3 dir: change the scope of function 'directory_exists_in_index()'Shourya Shukla, Oct 7, 2020
  3. Junio C HamanoOct 7, 2020
  4. Shourya ShuklaOct 12, 2020
  5. Emily ShafferNov 18, 2020
  6. 2/3 submodule: port submodule subcommand 'add' from shell to CShourya Shukla, Oct 7, 2020
  7. Junio C HamanoOct 7, 2020
  8. Junio C HamanoOct 7, 2020
  9. Junio C HamanoOct 8, 2020
  10. Junio C HamanoOct 9, 2020
  11. Jonathan TanNov 18, 2020
  12. Ævar Arnfjörð BjarmasonNov 19, 2020
  13. Johannes SchindelinNov 19, 2020
  14. Junio C HamanoNov 19, 2020
  15. 3/3 t7400: add test to check 'submodule add' for tracked pathsShourya Shukla, Oct 7, 2020
  16. Josh SteadmonNov 19, 2020

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.