Re: [GSoC][PATCH 3/4] submodule: port set_name_rev() from shell to C

On Mon, Aug 21, 2017 at 10:17 PM, Heiko Voigt [off-list ref] wrote:

On Mon, Aug 21, 2017 at 09:45:14PM +0530, Prathamesh Chavan wrote:

quoted

Function set_name_rev() is ported from git-submodule to the
submodule--helper builtin. The function get_name_rev() generates the
value of the revision name as required, and the function
print_name_rev() handles the formating and printing of the obtained
revision name.

Mentored-by: Christian Couder [off-list ref]
Mentored-by: Stefan Beller [off-list ref]
Signed-off-by: Prathamesh Chavan <redacted>
---
 builtin/submodule--helper.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            | 16 ++----------
 2 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7803457ba..a4bff3f38 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c

[...]

quoted

@@ -1242,6 +1304,7 @@ static struct cmd_struct commands[] = {
      {"relative-path", resolve_relative_path, 0},
      {"resolve-relative-url", resolve_relative_url, 0},
      {"resolve-relative-url-test", resolve_relative_url_test, 0},
+     {"print-name-rev", print_name_rev, 0},

I see the function in git-submodule.sh was named kind of reverse. How
about we do it more naturally here and call this 'rev-name' instead?
That makes is more clear to me and is also similar to the used variable
name 'revname'.

The functions 'print_name_rev()' and 'get_name_rev()' are the ported
C functions of the function 'set_name_rev()'
Their names were assigned so due to the existing function's name,
and hence to faithfully port the functions.

But, thanks for the above suggestion, and also for reviewing
the patch. I will update the names as print_rev_name() and
get_rev_name() respectively.

I would also prefix it differently like 'get' or 'calculate' instead of
'print' since it tries to find a name and is not just a simple lookup.

The former function from the shell script, 'set_name_rev()' is split
into two functions, namely: 'print_name_rev()' and 'get_name_rev()'
The function print_name_rev() is just the front_end of the function,
and exists to printf the return value of the get_name_rev() function
is the required format.
Calculation of the value is actually done by the function
get_name_rev().
Hence, I named the functions the way they are.

Thanks,
Prathamesh Chavan