git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kjetil Barvik <barvik@broadpark.no>
Cc: git@vger.kernel.org
Subject: Re: [PATCH/RFC v1 3/6] cleanup of write_entry() in entry.c
Date: Wed, 28 Jan 2009 13:31:54 -0800	[thread overview]
Message-ID: <7vy6wvkth1.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 1233004637-15112-4-git-send-email-barvik@broadpark.no

Kjetil Barvik <barvik@broadpark.no> writes:

> The switch-cases for S_IFREG and S_IFLNK was so similar that it will
> be better to do some cleanup and use the common parts of it.
>
> Also fold the longest lines such that no line is longer then 80 chars
> or so.
>
> And the entry.c file should now be clean for 'gcc -Wextra' warnings.
>
> Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
> ---
>
> If people do not like this approach I can be willing to drop it from
> this patch-series, but then I get some source code duplication from
> the next patch (4/6).

Merging of the two similar codepaths looked good from a cursory reading,
but if you have doubts about one patch, it usually is easier for other
people to work with you if the series is reordered to have it near the
end, iow, undisputably good ones first.

> -			return error("git checkout-index: unable to read sha1 file of %s (%s)",
> -				path, sha1_to_hex(ce->sha1));
> +			return error("git checkout-index: "\
> +				     "unable to read sha1 file of %s (%s)",
> +				     path, sha1_to_hex(ce->sha1));

You lost greppability when somebody calls for help saying "I am getting an
error message that says 'git checkout-index: unable to read sha1 file'".

  reply	other threads:[~2009-01-28 21:33 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-26 21:17 [PATCH/RFC v1 0/6] git checkout: more cleanups, optimisation, less lstat() calls Kjetil Barvik
2009-01-26 21:17 ` [PATCH/RFC v1 1/6] symlinks.c: small cleanup and optimisation Kjetil Barvik
2009-01-28 20:36   ` ??? " Junio C Hamano
2009-01-29 14:19     ` Kjetil Barvik
2009-01-26 21:17 ` [PATCH/RFC v1 2/6] remove some memcpy() and strchr() calls inside create_directories() Kjetil Barvik
2009-01-28 20:36   ` Junio C Hamano
2009-01-26 21:17 ` [PATCH/RFC v1 3/6] cleanup of write_entry() in entry.c Kjetil Barvik
2009-01-28 21:31   ` Junio C Hamano [this message]
2009-01-26 21:17 ` [PATCH/RFC v1 4/6] use fstat() instead of lstat() when we have an opened file Kjetil Barvik
2009-01-26 21:17 ` [PATCH/RFC v1 5/6] combine-diff.c: remove a call to fstat() inside show_patch_diff() Kjetil Barvik
2009-01-27  9:35   ` Mike Ralphson
2009-01-27 12:03     ` Kjetil Barvik
2009-01-27 12:06       ` Mike Ralphson
2009-01-28 20:37   ` Junio C Hamano
2009-01-29  1:16     ` Junio C Hamano
2009-01-29  8:20       ` Kjetil Barvik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vy6wvkth1.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=barvik@broadpark.no \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).