From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Johan Herland <johan@herland.net>
Cc: git@vger.kernel.org, Alex Riesen <raa.lkml@gmail.com>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 07/21] Copy the remaining differences from verify_tag() to parse_tag_buffer_internal()
Date: Sun, 10 Jun 2007 09:22:28 +0100 (BST) [thread overview]
Message-ID: <Pine.LNX.4.64.0706100917290.4059@racer.site> (raw)
In-Reply-To: <200706092339.12821.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> I'm probably splitting this up into too small pieces, since I keep
> getting comments that fail to see the overall picture of what I'm trying
> to do, [...]
Maybe I said that your patch was too large. But then, I said something
much more important: hard-to-review.
These small patches, split in a manner making it even more difficult to
understand what you want to accomplish, do not help.
Yes, you should make small patches. Even small patch series. But in such a
fashion that a reviewer can see that it is a good patch[*1*]. Just lean
back, look at your patches, and ask yourself how you would have reacted if
you had reviewed them.
Ciao,
Dscho
*1* A good patch follows the immortal words of Saint Exupery: A designer
knows he has achieved perfection not when there is nothing left to add,
but when there is nothing left to take away.
next prev parent reply other threads:[~2007-06-10 8:25 UTC|newest]
Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-07 22:50 error: char103: premature end of data Johannes Schindelin
2007-06-07 23:05 ` Johan Herland
2007-06-07 23:28 ` Johannes Schindelin
2007-06-07 23:47 ` Johan Herland
2007-06-07 23:55 ` Johannes Schindelin
2007-06-08 0:08 ` [PATCH] Fix failed tag parsing when tag object has no body/message (and thus ends with a single '\n') Johan Herland
2007-06-08 6:05 ` Junio C Hamano
2007-06-08 8:18 ` Johan Herland
2007-06-08 16:06 ` Junio C Hamano
2007-06-09 0:10 ` [PATCH 0/21] Refactor the tag object (take 2) Johan Herland
2007-06-09 0:12 ` [PATCH 01/21] Remove unnecessary code and comments on non-existing 8kB tag object restriction Johan Herland
2007-06-09 0:13 ` [PATCH 02/21] Return error messages when parsing fails Johan Herland
2007-06-09 18:01 ` Junio C Hamano
2007-06-09 18:28 ` Johan Herland
2007-06-09 19:42 ` [PATCH] Silence error messages unless 'thorough_verify' is set Johan Herland
2007-06-10 6:48 ` Johannes Schindelin
2007-06-10 8:15 ` Junio C Hamano
2007-06-10 10:08 ` Johannes Schindelin
2007-06-10 12:10 ` Johan Herland
2007-06-10 18:51 ` Johannes Schindelin
2007-06-10 19:16 ` Johan Herland
2007-06-10 11:47 ` [PATCH 0/4] Restructure the tag object Johan Herland
2007-06-10 11:49 ` [PATCH 1/4] Make tag names (i.e. the tag object's "tag" line) optional Johan Herland
2007-06-10 22:46 ` Junio C Hamano
2007-06-10 23:01 ` Johan Herland
2007-06-11 1:11 ` Junio C Hamano
2007-06-10 11:50 ` [PATCH 2/4] Introduce optional "keywords" on tag objects Johan Herland
2007-06-10 18:42 ` Johannes Schindelin
2007-06-10 19:04 ` Johan Herland
2007-06-10 21:43 ` Junio C Hamano
2007-06-10 23:16 ` Johan Herland
2007-06-11 1:01 ` Junio C Hamano
2007-06-10 11:50 ` [PATCH 3/4] Documentation/git-mktag: Document the changes in tag object structure Johan Herland
2007-06-10 11:50 ` [PATCH 4/4] git-mktag tests: Expand on mktag selftests according to the new " Johan Herland
2007-06-10 18:35 ` [PATCH 0/4] Restructure the tag object Johannes Schindelin
2007-06-09 0:13 ` [PATCH 03/21] Refactoring to make verify_tag() and parse_tag_buffer() more similar Johan Herland
2007-06-09 2:54 ` Johannes Schindelin
2007-06-09 10:49 ` Johan Herland
2007-06-09 0:14 ` [PATCH 04/21] Refactor verification of "tagger" line to be more similar to verification of "type" and "tagger" lines Johan Herland
2007-06-09 18:01 ` Junio C Hamano
2007-06-10 7:49 ` Johannes Schindelin
2007-06-09 0:14 ` [PATCH 05/21] Make parse_tag_buffer_internal() handle item == NULL Johan Herland
2007-06-09 18:01 ` Junio C Hamano
2007-06-10 0:45 ` [PATCH] Move check for already parsed tag object to parse_tag_buffer() wrapper function Johan Herland
2007-06-10 8:06 ` [PATCH 05/21] Make parse_tag_buffer_internal() handle item == NULL Johannes Schindelin
2007-06-09 0:15 ` [PATCH 06/21] Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line' Johan Herland
2007-06-09 21:26 ` Alex Riesen
2007-06-09 21:34 ` Johan Herland
2007-06-10 8:14 ` Johannes Schindelin
2007-06-10 9:01 ` Johannes Schindelin
2007-06-09 0:15 ` [PATCH 07/21] Copy the remaining differences from verify_tag() to parse_tag_buffer_internal() Johan Herland
2007-06-09 21:31 ` Alex Riesen
2007-06-09 21:39 ` Johan Herland
2007-06-10 8:22 ` Johannes Schindelin [this message]
2007-06-09 0:15 ` [PATCH 08/21] Switch from verify_tag() to parse_and_verify_tag_buffer() for verifying tag objects in git-mktag Johan Herland
2007-06-09 0:16 ` [PATCH 09/21] Remove unneeded code from mktag.c Johan Herland
2007-06-09 21:39 ` Alex Riesen
2007-06-09 21:42 ` Johan Herland
2007-06-09 0:16 ` [PATCH 10/21] Free mktag's buffer before dying Johan Herland
2007-06-09 21:37 ` Alex Riesen
2007-06-09 21:46 ` Johan Herland
2007-06-09 22:00 ` Alex Riesen
2007-06-09 22:05 ` Johan Herland
2007-06-10 8:38 ` Johannes Schindelin
2007-06-09 0:17 ` [PATCH 11/21] Rewrite error messages; fix up line lengths Johan Herland
2007-06-10 8:38 ` Johannes Schindelin
2007-06-09 0:17 ` [PATCH 12/21] Use prefixcmp() instead of memcmp() for cleaner code with less magic numbers Johan Herland
2007-06-09 21:42 ` Alex Riesen
2007-06-09 21:47 ` Johan Herland
2007-06-10 8:41 ` Johannes Schindelin
2007-06-09 0:18 ` [PATCH 13/21] Collect skipping of header field names and calculation of line lengths in one place Johan Herland
2007-06-10 8:45 ` Johannes Schindelin
2007-06-09 0:18 ` [PATCH 14/21] Add proper parsing of "tagger" line, but only when thorough_verify is set Johan Herland
2007-06-10 8:52 ` Johannes Schindelin
2007-06-10 8:58 ` Johannes Schindelin
2007-06-09 0:19 ` [PATCH 15/21] Make tag names (i.e. the tag object's "tag" line) optional Johan Herland
2007-06-10 9:07 ` Johannes Schindelin
2007-06-09 0:19 ` [PATCH 16/21] Introduce optional "keywords" on tag objects Johan Herland
2007-06-09 21:52 ` Alex Riesen
2007-06-09 22:00 ` Johan Herland
2007-06-09 22:36 ` [PATCH] Use xstrndup() instead of xmalloc() and memcpy(); fix buglet with generating default item->keywords Johan Herland
2007-06-10 0:05 ` [PATCH 16/21] Introduce optional "keywords" on tag objects Junio C Hamano
2007-06-10 0:35 ` [PATCH] Fail if tag name and keywords is not within "printable ASCII" Johan Herland
2007-06-10 1:33 ` Junio C Hamano
2007-06-09 0:20 ` [PATCH 17/21] Update comments on tag objects in mktag.c Johan Herland
2007-06-09 0:20 ` [PATCH 18/21] git-fsck: Do thorough verification of tag objects Johan Herland
2007-06-09 0:20 ` [PATCH 19/21] Documentation/git-mktag: Document the changes in tag object structure Johan Herland
2007-06-09 0:21 ` [PATCH 20/21] git-mktag tests: Expand on mktag selftests according to the new " Johan Herland
2007-06-09 0:21 ` [PATCH 21/21] Add fsck_verify_ref_to_tag_object() to verify that refname matches name stored in tag object Johan Herland
2007-06-07 23:11 ` error: char103: premature end of data Johannes Schindelin
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=Pine.LNX.4.64.0706100917290.4059@racer.site \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johan@herland.net \
--cc=raa.lkml@gmail.com \
/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).