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

Re: [PATCH v2 2/8] config: Reorganize get_color*.

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 17, 2009, 02:24 UTC
Message-ID
<7v63j9by1s.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<1234832094-15541-2-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> In preparation for parseopt.

I like this patch, because it clarifies what get_colorbool() and get_color() functions are meant to do by moving the boundary of responsibility between the two callers and these two functions.

The above log message does not do justice to the patch text.
>  builtin-config.c |   63 +++++++++++++++--------------------------------------
>  1 files changed, 18 insertions(+), 45 deletions(-)

I like a patch that results in code reduction, so I got quite interested in seeing what you did. But there was no magic --- you lost a lot of comments on what each function is supposed to do.

They are all described in the documentation, and removal of these comments that can go stale is probably a good thing, but you could have avoided dissapointing me who expected a magic by mentioning the removal of the comments (and why it is a good idea) upfront ;-)

Previous: Felipe ContrerasNext: Junio C Hamano
Message 18 of 24 in “config: Trivial rename in preparation for parseopt.”
  1. 1/8 config: Trivial rename in preparation for parseopt.Felipe Contreras, Feb 17, 2009
  2. 2/8 config: Reorganize get_color*.Felipe Contreras, Feb 17, 2009
  3. 3/8 config: Use parseopt.Felipe Contreras, Feb 17, 2009
  4. 4/8 config: Disallow multiple variable types.Felipe Contreras, Feb 17, 2009
  5. 5/8 config: Disallow multiple config file locations.Felipe Contreras, Feb 17, 2009
  6. 6/8 config: Don't allow extra arguments for -e or -l.Felipe Contreras, Feb 17, 2009
  7. 7/8 config: Codestyle cleanups.Felipe Contreras, Feb 17, 2009
  8. 8/8 config: Cleanup editor action.Felipe Contreras, Feb 17, 2009
  9. Junio C HamanoFeb 17, 2009
  10. Junio C HamanoFeb 17, 2009
  11. Junio C HamanoFeb 17, 2009
  12. Felipe ContrerasFeb 17, 2009
  13. Junio C HamanoFeb 17, 2009
  14. Felipe ContrerasFeb 17, 2009
  15. Johannes SchindelinFeb 17, 2009
  16. Felipe ContrerasFeb 17, 2009
  17. Felipe ContrerasFeb 17, 2009
  18. Junio C HamanoFeb 17, 2009
  19. Junio C HamanoFeb 17, 2009
  20. Felipe ContrerasFeb 17, 2009
  21. Johannes SchindelinFeb 17, 2009
  22. Felipe ContrerasFeb 17, 2009
  23. Gerrit PapeFeb 17, 2009
  24. Johannes SchindelinFeb 17, 2009

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.