| Message ID | 20200427204859.171084-1-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bugreport: drop time.h include | expand |
Emily Shaffer wrote: > As pointed out in > https://lore.kernel.org/git/20200425003002.GC17217@google.com, This breadcrumb shouldn't be needed, since the rest of the commit message already speaks for itself. We can save the future "git log" reader some time by leaing it out. > we don't > need to include time.h explicitly because it is included (and possibly > managed for portability) by cache.h. Drop the include. > > Signed-off-by: Emily Shaffer <emilyshaffer@google.com> > --- > Thanks for the observation, Jonathan. > > - Emily > > bugreport.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/bugreport.c b/bugreport.c > index 089b939a87..e4a7ed3a23 100644 > --- a/bugreport.c > +++ b/bugreport.c > @@ -2,7 +2,6 @@ > #include "parse-options.h" > #include "stdio.h" > #include "strbuf.h" > -#include "time.h" Same applies to stdio.h, I believe. Forgive my laziness in not being exhaustive before. Thanks, Jonathan
Jonathan Nieder <jrnieder@gmail.com> writes: > Emily Shaffer wrote: > >> As pointed out in >> https://lore.kernel.org/git/20200425003002.GC17217@google.com, > > This breadcrumb shouldn't be needed, since the rest of the commit > message already speaks for itself. We can save the future "git log" > reader some time by leaing it out. True. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > Jonathan Nieder <jrnieder@gmail.com> writes: > >> Emily Shaffer wrote: >> >>> As pointed out in >>> https://lore.kernel.org/git/20200425003002.GC17217@google.com, >> >> This breadcrumb shouldn't be needed, since the rest of the commit >> message already speaks for itself. We can save the future "git log" >> reader some time by leaing it out. > > True. Well, removing these two lines made the rest non-sentence, so I had to rewrite the thing. I am not sure if the educational value warrants the mention of compat/ exemption, but it people find it too noisy, it can certainly be dropped. Thanks. -- >8 -- From: Emily Shaffer <emilyshaffer@google.com> Date: Mon, 27 Apr 2020 13:48:59 -0700 Subject: [PATCH] bugreport: do not include <time.h> In the generic parts of the source files, system headers like <time.h> are supposed to be included indirectly by including "git-compat-util.h", which manages portability issues (platform specific compat/ sources are generally exempt from this rule). Drop the inclusion. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- bugreport.c | 1 - 1 file changed, 1 deletion(-) diff --git a/bugreport.c b/bugreport.c index 089b939a87..e4a7ed3a23 100644 --- a/bugreport.c +++ b/bugreport.c @@ -2,7 +2,6 @@ #include "parse-options.h" #include "stdio.h" #include "strbuf.h" -#include "time.h" #include "help.h" #include "compat/compiler.h"
On Mon, Apr 27, 2020 at 02:41:12PM -0700, Junio C Hamano wrote: > > Junio C Hamano <gitster@pobox.com> writes: > > > Jonathan Nieder <jrnieder@gmail.com> writes: > > > >> Emily Shaffer wrote: > >> > >>> As pointed out in > >>> https://lore.kernel.org/git/20200425003002.GC17217@google.com, > >> > >> This breadcrumb shouldn't be needed, since the rest of the commit > >> message already speaks for itself. We can save the future "git log" > >> reader some time by leaing it out. > > > > True. > > Well, removing these two lines made the rest non-sentence, so I had > to rewrite the thing. I am not sure if the educational value warrants > the mention of compat/ exemption, but it people find it too noisy, > it can certainly be dropped. I've got a reroll to drop the "stdio.h" include too - do you want me to send it? Your commit message is much nicer than what I came up with on my end dropping the breadcrumb and generalizing to include stdio.h, so I can adapt it if you're interested in the reroll. > > Thanks. > > -- >8 -- > > In the generic parts of the source files, system headers like > <time.h> are supposed to be included indirectly by including > "git-compat-util.h", which manages portability issues (platform > specific compat/ sources are generally exempt from this rule). > > Drop the inclusion. > > Signed-off-by: Emily Shaffer <emilyshaffer@google.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > bugreport.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/bugreport.c b/bugreport.c > index 089b939a87..e4a7ed3a23 100644 > --- a/bugreport.c > +++ b/bugreport.c > @@ -2,7 +2,6 @@ > #include "parse-options.h" > #include "stdio.h" > #include "strbuf.h" > -#include "time.h" > #include "help.h" > #include "compat/compiler.h" > > -- > 2.26.2-266-ge870325ee8 >
Emily Shaffer <emilyshaffer@google.com> writes: > On Mon, Apr 27, 2020 at 02:41:12PM -0700, Junio C Hamano wrote: >> >> Junio C Hamano <gitster@pobox.com> writes: >> >> > Jonathan Nieder <jrnieder@gmail.com> writes: >> > >> >> Emily Shaffer wrote: >> >> >> >>> As pointed out in >> >>> https://lore.kernel.org/git/20200425003002.GC17217@google.com, >> >> >> >> This breadcrumb shouldn't be needed, since the rest of the commit >> >> message already speaks for itself. We can save the future "git log" >> >> reader some time by leaing it out. >> > >> > True. >> >> Well, removing these two lines made the rest non-sentence, so I had >> to rewrite the thing. I am not sure if the educational value warrants >> the mention of compat/ exemption, but it people find it too noisy, >> it can certainly be dropped. > > I've got a reroll to drop the "stdio.h" include too - do you want me to > send it? Your commit message is much nicer than what I came up with on > my end dropping the breadcrumb and generalizing to include stdio.h, so I > can adapt it if you're interested in the reroll. Sure. I already queued it and merged it to 'next', but the result hasn't been pushed out and I am bisecting a test failure in some newish topics that are merged to 'jch' recently, so I do not mind taking a reroll and redoing 'next' before I can push it out today. Thanks.
diff --git a/bugreport.c b/bugreport.c index 089b939a87..e4a7ed3a23 100644 --- a/bugreport.c +++ b/bugreport.c @@ -2,7 +2,6 @@ #include "parse-options.h" #include "stdio.h" #include "strbuf.h" -#include "time.h" #include "help.h" #include "compat/compiler.h"
As pointed out in https://lore.kernel.org/git/20200425003002.GC17217@google.com, we don't need to include time.h explicitly because it is included (and possibly managed for portability) by cache.h. Drop the include. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- Thanks for the observation, Jonathan. - Emily bugreport.c | 1 - 1 file changed, 1 deletion(-)