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

Re: [PATCH 7/7] Implement git commit as a builtin command.

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 18, 2007, 13:58 UTC
Message-ID
<Pine.LNX.4.64.0709181453220.28586@racer.site>
In-Reply-To
<11900740163661-git-send-email-krh@redhat.com>
Hi,
very nice!
Four nits, though, and a half:
- it would be nicer to put the option parsing it option.[ch] (you would 
  also need to pass the usage line then, instead of hardwiring it to 
  "git_commit_usage"),
- it seems more logical to me to call it "parse_option()" than 
  "scan_options()", since that is what it does,
- you might want to rename OPTION_NONE to OPTION_BOOLEAN, and maybe even 
  allow "--no-<option>" in that case for free,
- wt_status_prepare() could take a parameter "index_file", which would 
  default to git_path("index") when passed as NULL, and
- launch_editor() is defined in builtin-tag.c, which is not part of the 
  library, and therefore it would be technically more correct to either 
  move the function to editor.c (my preferred solution), or declare it in 
  builtin.h instead of strbuf.h.

As you can see, my nits are really minor, which means that I am pretty happy with your work!

Thanks, Dscho

Previous: Kristian HøgsbergNext: Kristian Høgsberg
Message 8 of 18 in “Enable wt-status output to a given FILE pointer.”
  1. 1/7 Enable wt-status output to a given FILE pointer.Kristian Høgsberg, Sep 18, 2007
  2. 2/7 Enable wt-status to run against non-standard index file.Kristian Høgsberg, Sep 18, 2007
  3. 3/7 Introduce entry point for launching add--interactive.Kristian Høgsberg, Sep 18, 2007
  4. 4/7 Clean up stripspace a bit, use strbuf even more.Kristian Høgsberg, Sep 18, 2007
  5. 5/7 Add strbuf_read_file().Kristian Høgsberg, Sep 18, 2007
  6. 6/7 Export rerere() and launch_editor().Kristian Høgsberg, Sep 18, 2007
  7. 7/7 Implement git commit as a builtin command.Kristian Høgsberg, Sep 18, 2007
  8. Johannes SchindelinSep 18, 2007
  9. Kristian HøgsbergSep 18, 2007
  10. Junio C HamanoSep 20, 2007
  11. Kristian HøgsbergSep 21, 2007
  12. Junio C HamanoSep 21, 2007
  13. Kristian HøgsbergSep 24, 2007
  14. Johannes SchindelinSep 18, 2007
  15. Junio C HamanoSep 19, 2007
  16. Kristian HøgsbergSep 21, 2007
  17. Johannes SchindelinSep 18, 2007
  18. Kristian HøgsbergSep 18, 2007

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.