Prathamesh Chavan <pc44...@gmail.com> writes:

> * The function get_submodule_displaypath() was modified for the case
>   when get_super_prefix() returns a non-null value. The condition to check
>   if the super-prefix ends with a '/' is removed. To accomodate this change
>   appropriate value of super_prefix is passed instead in the recursive calls
>   of init_submodule() and status_submodule().

OK.

> * To accomodate the possiblity of a direct call to the function
>   init_submodule(), a callback function init_submodule_cb() is introduced
>   which takes cache_entry and init_cb structures as input params, and
>   calls init_submodule() with parameters which are more appropriate
>   for a direct call of this function.

OK.

> * Similar changes were even done for status_submodule(). But as 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
>   cb_flags was introduced to store all of these flags, instead of having
>   parameter for each one.

Use of a flag word instead of many bit parameter is a very good
idea.  I do not think it is brilliant to call a field in a structure
that is used as an interface to the callback interface cb_flags,
though, especially when it is already clear that it is about the
callback from the name the structure.  Just name them "flags".

I may or may not leave more detailed comments on individual patches.

Thanks.

Reply via email to