On Mon, Aug 21, 2017 at 10:17 PM, Heiko Voigt <hvoigt@xxxxxxxxxx> wrote:
> On Mon, Aug 21, 2017 at 09:45:14PM +0530, Prathamesh Chavan wrote:
>> 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 <christian.couder@xxxxxxxxx>
>> Mentored-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> Signed-off-by: Prathamesh Chavan <pc44800@xxxxxxxxx>
>> ---
>> 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
>
> [...]
>
>> @@ -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
