| Message ID | 20200702192409.21865-2-shouryashukla.oo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | submodule: port 'summary' from Shell to C | expand |
Hi Shourya, "amend" sounds a little awkward to describe what the patch does. Maybe "submodule: remove extra empty lines"? On Fri, 3 Jul 2020, Shourya Shukla wrote: > All subcommands of 'git submodule' using a callback mechanism had > absence of an extra linefeed between their callback structs and > macros. Subcommands 'init', 'status' and 'sync' did not follow suit. > Amend the extra line feed. Maybe a native reader can suggest something that flows a bit easier? I am not a native English speaker, but I'd prefer something along those lines: Many `submodule--helper` subcommands follow the convention a struct defines their callback data, and the declaration of said struct is followed immediately by a macro to use in static initializers, without any separating empty line. Let's align the `init`, `status` and `sync` subcommands with that convention. The patch obviously does what the commit message promises. Ciao, Dscho > Mentored-by: Christian Couder <chriscool@tuxfamily.org> > Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> > Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com> > --- > builtin/submodule--helper.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 59c1e1217c..eea3932c40 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -612,7 +612,6 @@ struct init_cb { > const char *prefix; > unsigned int flags; > }; > - > #define INIT_CB_INIT { NULL, 0 } > > static void init_submodule(const char *path, const char *prefix, > @@ -742,7 +741,6 @@ struct status_cb { > const char *prefix; > unsigned int flags; > }; > - > #define STATUS_CB_INIT { NULL, 0 } > > static void print_status(unsigned int flags, char state, const char *path, > @@ -933,7 +931,6 @@ struct sync_cb { > const char *prefix; > unsigned int flags; > }; > - > #define SYNC_CB_INIT { NULL, 0 } > > static void sync_submodule(const char *path, const char *prefix, > -- > 2.27.0 > >
Suggestion... On 03/07/2020 15:57, Johannes Schindelin wrote: > Maybe a native reader can suggest something that flows a bit easier? I am > not a native English speaker, but I'd prefer something along those lines: > > Many `submodule--helper` subcommands follow the convention a s/convention a/convention that a/ feels nicer for me > struct defines their callback data, and the declaration of said s/said/that/ maybe. 'said' is a bit too much like 'patent speak', but otherwise either word is OK. > struct is followed immediately by a macro to use in static > initializers, without any separating empty line. > > Let's align the `init`, `status` and `sync` subcommands with that > convention. Nice summary. Philip Native Yorkshire speaker, but not that 'literate' ;-)
On 03/07 04:37, Philip Oakley wrote: > > Suggestion... > > On 03/07/2020 15:57, Johannes Schindelin wrote: > > Maybe a native reader can suggest something that flows a bit easier? I am > > not a native English speaker, but I'd prefer something along those lines: > > > > Many `submodule--helper` subcommands follow the convention a > s/convention a/convention that a/ feels nicer for me I did not get this one. Are you asking to replace "convention" with "a" only? > > struct defines their callback data, and the declaration of said > s/said/that/ maybe. 'said' is a bit too much like 'patent speak', but > otherwise either word is OK. Okay will do! > > struct is followed immediately by a macro to use in static > > initializers, without any separating empty line. > > > > Let's align the `init`, `status` and `sync` subcommands with that > > convention. > Nice summary. > > Philip > Native Yorkshire speaker, but not that 'literate' ;-) This made me chuckle. Nice!
On 2020-07-04 20:51:04+0530, Shourya Shukla <shouryashukla.oo@gmail.com> wrote: > On 03/07 04:37, Philip Oakley wrote: > > > > Suggestion... > > > > On 03/07/2020 15:57, Johannes Schindelin wrote: > > > Maybe a native reader can suggest something that flows a bit easier? I am > > > not a native English speaker, but I'd prefer something along those lines: > > > > > > Many `submodule--helper` subcommands follow the convention a > > s/convention a/convention that a/ feels nicer for me > > I did not get this one. Are you asking to replace "convention" with "a" > only? He probably meant put "that" between "convention" and "a" Pull ed/vi/sed out and that try this command on that line :) s/convention a/convention that a/
On 04/07/2020 16:39, Đoàn Trần Công Danh wrote: > On 2020-07-04 20:51:04+0530, Shourya Shukla <shouryashukla.oo@gmail.com> wrote: >> On 03/07 04:37, Philip Oakley wrote: >>> Suggestion... >>> >>> On 03/07/2020 15:57, Johannes Schindelin wrote: >>>> Maybe a native reader can suggest something that flows a bit easier? I am >>>> not a native English speaker, but I'd prefer something along those lines: >>>> >>>> Many `submodule--helper` subcommands follow the convention a >>> s/convention a/convention that a/ feels nicer for me >> I did not get this one. Are you asking to replace "convention" with "a" >> only? > He probably meant put "that" between "convention" and "a" > Pull ed/vi/sed out and that try this command on that line :) > > s/convention a/convention that a/ > > Correct. Philip
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 59c1e1217c..eea3932c40 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -612,7 +612,6 @@ struct init_cb { const char *prefix; unsigned int flags; }; - #define INIT_CB_INIT { NULL, 0 } static void init_submodule(const char *path, const char *prefix, @@ -742,7 +741,6 @@ struct status_cb { const char *prefix; unsigned int flags; }; - #define STATUS_CB_INIT { NULL, 0 } static void print_status(unsigned int flags, char state, const char *path, @@ -933,7 +931,6 @@ struct sync_cb { const char *prefix; unsigned int flags; }; - #define SYNC_CB_INIT { NULL, 0 } static void sync_submodule(const char *path, const char *prefix,
All subcommands of 'git submodule' using a callback mechanism had absence of an extra linefeed between their callback structs and macros. Subcommands 'init', 'status' and 'sync' did not follow suit. Amend the extra line feed. Mentored-by: Christian Couder <chriscool@tuxfamily.org> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com> --- builtin/submodule--helper.c | 3 --- 1 file changed, 3 deletions(-)