[1/4] submodule: amend extra line feed between callback struct and macro
diff mbox series

Message ID 20200702192409.21865-2-shouryashukla.oo@gmail.com
State New
Headers show
Series
  • submodule: port 'summary' from Shell to C
Related show

Commit Message

Shourya Shukla July 2, 2020, 7:24 p.m. UTC
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(-)

Comments

Johannes Schindelin July 3, 2020, 2:57 p.m. UTC | #1
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
>
>
Philip Oakley July 3, 2020, 3:37 p.m. UTC | #2
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' ;-)
Shourya Shukla July 4, 2020, 3:21 p.m. UTC | #3
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!
Đoàn Trần Công Danh July 4, 2020, 3:39 p.m. UTC | #4
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/
Philip Oakley July 5, 2020, 10:52 a.m. UTC | #5
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

Patch
diff mbox series

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,