| Message ID | patch-01.11-e51c860a65d-20210328T021238Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | improve reporting of unexpected objects | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Change the type_from_string() macro into a function and drop the > support for passing len < 0. > > Support for len < 0 was added in fe8e3b71805 (Refactor > type_from_string() to allow continuing after detecting an error, > 2014-09-10), but no callers use that form. Let's drop it to simplify > this, and in preparation for simplifying these even further. Given the recent fallout of oversimplifying we've seen in other topic, this line of thinking makes me nauseated, but let's see how well this works this time around. At least, replacing an already queued topic with v2 would not increase the number of topics that are supposedly in-flight but not quite moving due to lack of reviews and responses, unlike bunch of totally new patches ;-) Will replace. Thanks. > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > object.c | 10 +++++++--- > object.h | 2 +- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/object.c b/object.c > index 78343781ae7..65446172172 100644 > --- a/object.c > +++ b/object.c > @@ -39,9 +39,6 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle) > { > int i; > > - if (len < 0) > - len = strlen(str); > - > for (i = 1; i < ARRAY_SIZE(object_type_strings); i++) > if (!strncmp(str, object_type_strings[i], len) && > object_type_strings[i][len] == '\0') > @@ -53,6 +50,13 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle) > die(_("invalid object type \"%s\""), str); > } > > +int type_from_string(const char *str) > +{ > + size_t len = strlen(str); > + int ret = type_from_string_gently(str, len, 0); > + return ret; > +} > + > /* > * Return a numerical hash value between 0 and n-1 for the object with > * the specified sha1. n must be a power of 2. Please note that the > diff --git a/object.h b/object.h > index 59daadce214..3ab3eb193d3 100644 > --- a/object.h > +++ b/object.h > @@ -94,7 +94,7 @@ struct object { > > const char *type_name(unsigned int type); > int type_from_string_gently(const char *str, ssize_t, int gentle); > -#define type_from_string(str) type_from_string_gently(str, -1, 0) > +int type_from_string(const char *str); > > /* > * Return the current number of buckets in the object hashmap.
On Sun, Mar 28 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Change the type_from_string() macro into a function and drop the >> support for passing len < 0. >> >> Support for len < 0 was added in fe8e3b71805 (Refactor >> type_from_string() to allow continuing after detecting an error, >> 2014-09-10), but no callers use that form. Let's drop it to simplify >> this, and in preparation for simplifying these even further. > > Given the recent fallout of oversimplifying we've seen in other > topic, this line of thinking makes me nauseated, but let's see how > well this works this time around. Do you mean related to tree-walk.[ch]? But yeah, this step doesn't striclly need to be taken, but seemed worth it given that there's just 10 or so callers (none of which used this). > At least, replacing an already queued topic with v2 would not > increase the number of topics that are supposedly in-flight but not > quite moving due to lack of reviews and responses, unlike bunch of > totally new patches ;-) I'm not sure what to do to improve things in that area. I'm obviously for increasing the net velocity of my patches making it to master, but if it's held up my number of reviews a submission of Y won't necessarily make X worse, since people who've got an interest in Y will be different than those with an interest in X. But some of it's definitely on my end, e.g. re-rolls sometimes taking me longer than I'd prefer. It's a different activity to dissect outstanding reviews & re-roll than writing code, and sometimes I'm interested in one over the other... >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >> --- >> object.c | 10 +++++++--- >> object.h | 2 +- >> 2 files changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/object.c b/object.c >> index 78343781ae7..65446172172 100644 >> --- a/object.c >> +++ b/object.c >> @@ -39,9 +39,6 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle) >> { >> int i; >> >> - if (len < 0) >> - len = strlen(str); >> - >> for (i = 1; i < ARRAY_SIZE(object_type_strings); i++) >> if (!strncmp(str, object_type_strings[i], len) && >> object_type_strings[i][len] == '\0') >> @@ -53,6 +50,13 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle) >> die(_("invalid object type \"%s\""), str); >> } >> >> +int type_from_string(const char *str) >> +{ >> + size_t len = strlen(str); >> + int ret = type_from_string_gently(str, len, 0); >> + return ret; >> +} >> + >> /* >> * Return a numerical hash value between 0 and n-1 for the object with >> * the specified sha1. n must be a power of 2. Please note that the >> diff --git a/object.h b/object.h >> index 59daadce214..3ab3eb193d3 100644 >> --- a/object.h >> +++ b/object.h >> @@ -94,7 +94,7 @@ struct object { >> >> const char *type_name(unsigned int type); >> int type_from_string_gently(const char *str, ssize_t, int gentle); >> -#define type_from_string(str) type_from_string_gently(str, -1, 0) >> +int type_from_string(const char *str); >> >> /* >> * Return the current number of buckets in the object hashmap.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> At least, replacing an already queued topic with v2 would not >> increase the number of topics that are supposedly in-flight but not >> quite moving due to lack of reviews and responses, unlike bunch of >> totally new patches ;-) > > I'm not sure what to do to improve things in that area. > > I'm obviously for increasing the net velocity of my patches making it to > master, but if it's held up my number of reviews a submission of Y won't > necessarily make X worse, since people who've got an interest in Y will > be different than those with an interest in X. > > But some of it's definitely on my end, e.g. re-rolls sometimes taking me > longer than I'd prefer. It's a different activity to dissect outstanding > reviews & re-roll than writing code, and sometimes I'm interested in one > over the other... What I'd like to encourage contributors to think is the velocity in the whole project, not only their own patches. The changes proposed on the list would consume the review bandwidth, which is unfortunately not an infinite resource. To balance the supply and the consumption, one way might be to throttle incoming patches to restrict consumption and distribute the supply more evenly among authors. But a more desirable way that would benefit the community more would be to increase the supply. If all of those who consume the review bandwidth tip in by reviewing others' patches, not limited to the area they are interested in but more in the "I am not so familiar with the area, but I've been here long enough and know general principles, so let's polish your patch together" spirit, that would help the community greatly, I would think, by: - replenishing review bandwidth they consumed from the pool; - throttling their patch flow that consume review bandwidth (while they are reviewing others patches, they won't be throwing new patches at the list to consume even more review bandwidth); - helping the reviewers themselves become more familiar with the parts of the code they are not working in right now. I am reasonably sure I and a few others on the list are net suppliers of the reviewer bandwidth. I do not expect all the prolific contributors to become net suppliers; after all, designing and writing their own stuff is always fun. But I wish that the most prominent contributors in the community to be reviewing others' topics and ushering these topics to completion from time to time, and I am hoping to see that happen more. Thanks.
Junio C Hamano wrote: > I am reasonably sure I and a few others on the list are net > suppliers of the reviewer bandwidth. I do not expect all the > prolific contributors to become net suppliers; after all, designing > and writing their own stuff is always fun. But I wish that the most > prominent contributors in the community to be reviewing others' > topics and ushering these topics to completion from time to time, > and I am hoping to see that happen more. The problem is that the suppliers are a club who often agree on what code is not ready to be merged, which is most of it, and also agree it's better to apply the reject hammer. This club is by definition small. There's a spectrum of perfectness, and the suppliers are on the far left side: code has to be perfect, while most of the consumers are on the right side, or even on the sensible middle side: do not let the perfect be the enemy of the good. For the suppliers club good is usually not good enough. I'm fairly confident most of the consumers would agree the bar on what constitutes "good enough" is too damn high, so why would they spend time raising it any higher? They won't. If anything they are more often going to dissagree with the suppliers club, in order to increase the likelihood of perfectly good patches to be merged. As long as you keep insisting on making the perfect being the enemy of the good, you are going to ensure the supply is *always* going to be low. Cheers.
diff --git a/object.c b/object.c index 78343781ae7..65446172172 100644 --- a/object.c +++ b/object.c @@ -39,9 +39,6 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle) { int i; - if (len < 0) - len = strlen(str); - for (i = 1; i < ARRAY_SIZE(object_type_strings); i++) if (!strncmp(str, object_type_strings[i], len) && object_type_strings[i][len] == '\0') @@ -53,6 +50,13 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle) die(_("invalid object type \"%s\""), str); } +int type_from_string(const char *str) +{ + size_t len = strlen(str); + int ret = type_from_string_gently(str, len, 0); + return ret; +} + /* * Return a numerical hash value between 0 and n-1 for the object with * the specified sha1. n must be a power of 2. Please note that the diff --git a/object.h b/object.h index 59daadce214..3ab3eb193d3 100644 --- a/object.h +++ b/object.h @@ -94,7 +94,7 @@ struct object { const char *type_name(unsigned int type); int type_from_string_gently(const char *str, ssize_t, int gentle); -#define type_from_string(str) type_from_string_gently(str, -1, 0) +int type_from_string(const char *str); /* * Return the current number of buckets in the object hashmap.
Change the type_from_string() macro into a function and drop the support for passing len < 0. Support for len < 0 was added in fe8e3b71805 (Refactor type_from_string() to allow continuing after detecting an error, 2014-09-10), but no callers use that form. Let's drop it to simplify this, and in preparation for simplifying these even further. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- object.c | 10 +++++++--- object.h | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-)