Another version, this time very in line with the review and commentary of
Junio, Eric, and Michael. This version boasts a revamped commit message and
fewer but surer hunks changed.
Thanks again for the guidance.
Quint Guvernator (1):
use starts_with() instead of !memcmp()
builtin/apply.c
When checking if a string begins with a constant string, using
starts_with() indicates the intention of the check more clearly and is
less error-prone than calling !memcmp() with an explicit byte count.
Signed-off-by: Quint Guvernator
---
builtin/apply.c| 4 ++--
builtin/for-each-ref.c
2014-03-17 21:59 GMT-04:00 Eric Sunshine :
> I can't speak for Junio, but the description could be made more
> concise and to-the-point. Aside from using imperative voice, you can
> eliminate redundancy, some of which comes from repeating in prose what
> the patch itself already states more concise
2014-03-17 18:52 GMT-04:00 Junio C Hamano :
> Thanks. This probably needs retitled, though (hint: "replaces"?
> who does so?) and the message rewritten (see numerous reviews on
> other GSoC micros from Eric).
I found some messages [1] by Eric concerning imperative voice ("simplify"
rather than "s
My mistake, folks. This is [PATCH v4]. Apologies for the confusion.
Quint
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
memcmp() is replaced with negated starts_with() when comparing strings
from the beginning and when it is logical to do so. starts_with() looks
nicer and it saves the extra argument of the length of the comparing
string.
Signed-off-by: Quint Guvernator
---
builtin/apply.c| 6
rply; Michael says:
> It would be much better for you to submit only a handful of changes (or
> even only one!) that is perfect, rather than throwing a bunch at the wall
> and asking the reviewer to pick between the good and the bad.
Thanks for the guidance, everyone.
This work is
2014-03-17 15:27 GMT-04:00 Eric Sunshine :
> A quick (perhaps inaccurate) search of the mailing list shows that, of
> the remaining "untaken" items, #10, 11, 12, 15, 16, and 18 have had
> just one submission, and #13 had two, so we're okay.
I am still working on #14: "Change fetch-pack.c:filter_re
2014-03-17 12:29 GMT-04:00 Michael Haggerty :
> This hunk uses the magic number "11" a couple lines later. Junio (I
> think rightly) objected [1] to rewrites in these circumstances because
> they make it even less obvious where the constant came from and thereby
> make the complete elimination of
Hi again, Git community!
My name is Quint Guvernator, and I am participating in the Google
Summer of Code program. I am in university at the College of William
and Mary in Williamsburg, VA and plan to major in Computer Science and
Linguistics.
I have been working on a microproject [1][2] to get
memcmp() is replaced with negated starts_with() when comparing strings
from the beginning and when it is logical to do so. starts_with() looks
nicer and it saves the extra argument of the length of the comparing
string.
Signed-off-by: Quint Guvernator
---
builtin/apply.c
again for your help. This is my first patch here, and has been
quite a microproject indeed!
Quint Guvernator (1):
general style: replaces memcmp() with starts_with()
builtin/apply.c | 6 +++---
builtin/for-each-ref.c| 2 +-
builtin/get-tar-c
2014-03-13 12:05 GMT-04:00 Michael Haggerty :
> It is very, very unlikely that you inverted the sense of dozens of tests
> throughout the Git code base and the tests ran correctly. I rather
> think that you made a mistake when testing. You should double- and
> triple-check that you really ran the
2014-03-14 0:57 GMT-04:00 Jeff King :
> This discussion ended up encompassing a lot of other related cleanups. I
> hope we didn't scare you away. :)
I don't think you could; this community is much more accepting than
other software communities around the web. The fact that I received
constructive
>From what I can gather, there seems to be opposition to specific
pieces of this patch.
The following area is clearly the most controversial:
> static inline int standard_header_field(const char *field, size_t len)
> {
> -return ((len == 4 && !memcmp(field, "tree ", 5)) ||
> -(l
2014-03-12 11:47 GMT-04:00 Jens Lehmann :
> I think this hunk should be dropped as the memcmp() here doesn't mean
> "starts with" but "is identical" (due to the "ce_namelen(ce) == 11" in
> the line above).
There is an issue with negation in this patch. I've submitted a new
one [1] to the mailing l
memcmp() is replaced with negated starts_with() when comparing strings
from the beginning. starts_with() looks nicer and it saves the extra
argument of the length of the comparing string.
note: this commit properly handles negation in starts_with()
Signed-off-by: Quint Guvernator
---
builtin
2014-03-12 9:51 GMT-04:00 Duy Nguyen :
> starts_with(..) == !memcmp(...). So
> you need to negate every replacement.
My apologies--it doesn't look like the tests caught it either. I will
fix this and submit a new patch.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body
memcmp() is replaced with starts_with() when comparing strings from the
beginning. starts_with() looks nicer and it saves the extra argument of
the length of the comparing string.
Signed-off-by: Quint Guvernator
---
builtin/apply.c | 10 +-
builtin/cat-file.c
19 matches
Mail list logo
