git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status

From
Stefan Beller <sbeller@google.com>
Date
Jun 5, 2017, 23:12 UTC
Message-ID
<CAGZ79kZW=B1kd91y6VCushz5tqk1jYW=WzHheHi1pSiMWMQ4iQ@mail.gmail.com>
In-Reply-To
<20170605202529.22959-2-pc44800@gmail.com>
On Mon, Jun 5, 2017 at 1:25 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
Show 36 quoted lines
> This aims to make git-submodule subcommand status a builtin. Here
> 'status' is ported to submodule--helper, and submodule--helper is
> called from git-submodule.sh.
>
> For the purpose of porting cmd_status, the code is split up such that
> one function obtains all the list of submodules, acting as the front-end
> of git-submodule status. This function later calls the second function
> for_each_submodule_list,it which basically loops through the list of
> submodules and calls function fn, which in this case is status_submodule.
> The third function, status submodule returns the status of submodule and
> also takes care of the recursive flag.
>
> The first function module_status parses the options present in argv,
> and then with the help of module_list_compute, generates the list of
> submodules present in the current working tree.
>
> The second function for_each_submodule_list traverses through the list,
> and calls function fn (which in the case of submodule subcommand status
> is status_submodule) is called for each entry.
>
> The third function status_submodule checks for the various conditions,
> and prints the status of the submodule accordingly. Also, this function
> takes care of the recursive flag by creating a separate child_process
> and running it inside the submodule. The function print_status handles the
> printing of submodule's status.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> ---
> In this new version of patch, function print_status is introduced.
>
> The functions for_each_submodule_list and get_submodule_displaypath
> are found to be the same as those in the ported submodule subcommand
> foreach's patches. The reason for doing so is to keep both the patches
> independant and on separate branches.
Maybe keep it even in a separate patch, such that
the status series becomes:
  patch 1: introduce for_each_submodule_list and get_submodule_displaypath
  patch 2: port print_name_rev
  patch 3: port status

whereas the foreach series (and other series later) could re-use patch 1, and build on top of it.

For reviewing patches, it is fine to have the get_submodule_displaypath is both series, though for applying patches it for less complication/deduplication from the maintainer I would think.

Show 16 quoted lines
> +
> +static void print_status(struct status_cb *info, char state, const char *path,
> +                        char *sub_sha1, char *displaypath)
> +{
> +       if (info->quiet)
> +               return;
> +
> +       printf("%c%s %s", state, sub_sha1, displaypath);
> +
> +       if (state == ' ' || state == '+') {
> +               struct argv_array name_rev_args = ARGV_ARRAY_INIT;
> +
> +               argv_array_pushl(&name_rev_args, "print-name-rev",
> +                                path, sub_sha1, NULL);
> +               print_name_rev(name_rev_args.argc, name_rev_args.argv,
> +                              info->prefix);

... with the suggestion given in the print_name_rev patch, this would become a one liner. :)

The rest looks good to me :)
Previous: Prathamesh ChavanNext: Christian Couder
Message 6 of 9 in “submodule: port set_name_rev from shell to C”
  1. Prathamesh ChavanMay 21, 2017
  2. [GSoC][PATCH v1 2/2] submodule: port submodule subcommand statusPrathamesh Chavan, May 21, 2017
  3. Stefan BellerMay 22, 2017
  4. [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to CPrathamesh Chavan, Jun 5, 2017
  5. [GSoC][PATCH v2 2/2] submodule: port submodule subcommand statusPrathamesh Chavan, Jun 5, 2017
  6. Stefan BellerJun 5, 2017
  7. Christian CouderJun 6, 2017
  8. Stefan BellerJun 5, 2017
  9. Brandon WilliamsJun 5, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.