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 quintus.pub...@gmail.com
---
builtin/apply.c| 4
2014-03-17 12:29 GMT-04:00 Michael Haggerty mhag...@alum.mit.edu:
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
2014-03-17 15:27 GMT-04:00 Eric Sunshine sunsh...@sunshineco.com:
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
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 microproject #14 for GSoC.
Quint Guvernator (1
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 quintus.pub...@gmail.com
---
builtin/apply.c
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
2014-03-17 18:52 GMT-04:00 Junio C Hamano gits...@pobox.com:
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
2014-03-17 21:59 GMT-04:00 Eric Sunshine sunsh...@sunshineco.com:
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
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
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-commit-id.c
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 quintus.pub...@gmail.com
---
builtin/apply.c
2014-03-14 0:57 GMT-04:00 Jeff King p...@peff.net:
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
2014-03-13 12:05 GMT-04:00 Michael Haggerty mhag...@alum.mit.edu:
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
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 quintus.pub...@gmail.com
---
builtin/apply.c | 10
2014-03-12 9:51 GMT-04:00 Duy Nguyen pclo...@gmail.com:
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
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 quintus.pub
2014-03-12 11:47 GMT-04:00 Jens Lehmann jens.lehm...@web.de:
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
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)) ||
-(len == 6
19 matches
Mail list logo
