Re: [PATCH v1 0/2] Incremental rewrite of git-submodules

[Brandon Williams <bmwill@google.com>]

On 01/09, Prathamesh Chavan wrote:
> The patches [1] and [2] concerning the porting of submodule
> subcommands: sync and deinit were updated in accoudance with
> the changes made in one of such similar portings made earlier
> for submodule subcommand status[3]. Following are the changes
> made:

The two patches look good to me.  Thanks for continuing this work!

> 
> * It was observed that the number of params increased a lot due to flags
>   like quiet, recursive, cached, etc, and keeping in mind the future
>   subcommand's ported functions as well, a single unsigned int called
>   flags was introduced to store all of these flags, instead of having
>   parameter for each one.

This is unfortunate.  The use of a flag word or using bit-fields are
essentially equivalent so its unfortunate that the conversion to using
one or the other caused review churn.  My own preference would be to use
bit-fields ;)  I also noticed that the flags you are using start with
OPT_* which conflict with the parse-options namespace, sorry for not
catching this when a few of your older patches made it into master.
This isn't a big deal since no symbols look to collide so I am not
suggesting you change this since I would prefer to eliminate more
unnecessary review churn on this series.

> 
> * To accomodate the possiblity of a direct call to the functions
>   deinit_submodule() and sync_submodule(), callback functions were
>   introduced.
> 
> As before you can find this series at: 
> https://github.com/pratham-pc/git/commits/patch-series-2
> 
> And its build report is available at: 
> https://travis-ci.org/pratham-pc/git/builds/
> Branch: patch-series-2
> Build #195
> 
> [1]: https://public-inbox.org/git/20170807211900.15001-6-pc44800@gmail.com/
> [2]: https://public-inbox.org/git/20170807211900.15001-7-pc44800@gmail.com/
> [3]: https://public-inbox.org/git/20171006132415.2876-4-pc44800@gmail.com/
> 
> Prathamesh Chavan (2):
>   submodule: port submodule subcommand 'sync' from shell to C
>   submodule: port submodule subcommand 'deinit' from shell to C
> 
>  builtin/submodule--helper.c | 345 ++++++++++++++++++++++++++++++++++++++++++++
>  git-submodule.sh            | 112 +-------------
>  2 files changed, 347 insertions(+), 110 deletions(-)
> 
> -- 
> 2.14.2
> 

-- 
Brandon Williams