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