Re: [PATCH 4/4] fast-export: trivial cleanup
On Wed, May 8, 2013 at 8:16 PM, Felipe Contreras wrote: > Cast the object to a commit, only to get the object back? I'm dropping this one. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] fast-export: trivial cleanup
John Szakmeister writes:
> On Thu, May 9, 2013 at 4:53 AM, Felipe Contreras
> wrote:
> [snip]
@@ -562,9 +561,7 @@ static void handle_tags_and_duplicates(struct
string_list *extra_refs)
break;
case OBJ_COMMIT:
/* create refs pointing to already seen commits */
- commit = (struct commit *)object;
- printf("reset %s\nfrom :%d\n\n", name,
- get_object_mark(&commit->object));
+ printf("reset %s\nfrom :%d\n\n", name,
get_object_mark(object));
>>>
>>> FWIW, this line is now too long (exceeds 80 columns). Good catch on
>>> the casting though.
>> ...
>> The key word being *try*.
>
> I saw that, but you actively joined the lines, and there was no need
> to. It didn't even require trying to keep it within 80 columns. :-)
That is somewhat a flawed argument.
Sometimes it makes it easier to have a single line that is slightly
longer than 80-col than split it across two lines. "Now one of the
arguments to this function invocation is much shorter. The only
reason the invocation originally was split into two lines was
because its argument list was too long, so joining them into a
single line _might_ make it easier to read. Let's try to see how it
reads." is a reasonable line of thought, and at that point, it
starts to require trying to keep it under 80-col.
In this particular case, however, I do not think that the single
line is particularly easier to read, but I do not think the original
is better, either. The easiest to read would be more like this:
printf("reset %s\nfrom :%d\n\n",
name, get_object_mark(object));
to show which argument corresponds to which %conversion [*1*]
[Footnote]
*1* When the format string has too many embedded LFs, showing it
like this:
printf("reset %s\n"
"from :%d\n\n",
name, get_object_mark(object));
is even better, but I do not think it applies to this particular
case.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] fast-export: trivial cleanup
On Thu, May 9, 2013 at 4:53 AM, Felipe Contreras
wrote:
[snip]
>>> @@ -562,9 +561,7 @@ static void handle_tags_and_duplicates(struct
>>> string_list *extra_refs)
>>> break;
>>> case OBJ_COMMIT:
>>> /* create refs pointing to already seen commits */
>>> - commit = (struct commit *)object;
>>> - printf("reset %s\nfrom :%d\n\n", name,
>>> - get_object_mark(&commit->object));
>>> + printf("reset %s\nfrom :%d\n\n", name,
>>> get_object_mark(object));
>>
>> FWIW, this line is now too long (exceeds 80 columns). Good catch on
>> the casting though.
>>
>> -John
>>
>> PS Sorry for the duplicate Felipe... I still need to get used to
>> hitting "Reply All". :-)
>
> The guideline is:
>
> - We try to keep to at most 80 characters per line.
>
> The key word being *try*.
I saw that, but you actively joined the lines, and there was no need
to. It didn't even require trying to keep it within 80 columns. :-)
-John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] fast-export: trivial cleanup
On Thu, May 9, 2013 at 3:52 AM, John Szakmeister wrote:
> On Wed, May 8, 2013 at 9:16 PM, Felipe Contreras
> wrote:
>> Cast the object to a commit, only to get the object back?
>>
>> Signed-off-by: Felipe Contreras
>> ---
>> builtin/fast-export.c | 5 +
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
>> index 8091354..d24b4d9 100644
>> --- a/builtin/fast-export.c
>> +++ b/builtin/fast-export.c
>> @@ -550,7 +550,6 @@ static void get_tags_and_duplicates(struct
>> rev_cmdline_info *info,
>>
>> static void handle_tags_and_duplicates(struct string_list *extra_refs)
>> {
>> - struct commit *commit;
>> int i;
>>
>> for (i = extra_refs->nr - 1; i >= 0; i--) {
>> @@ -562,9 +561,7 @@ static void handle_tags_and_duplicates(struct
>> string_list *extra_refs)
>> break;
>> case OBJ_COMMIT:
>> /* create refs pointing to already seen commits */
>> - commit = (struct commit *)object;
>> - printf("reset %s\nfrom :%d\n\n", name,
>> - get_object_mark(&commit->object));
>> + printf("reset %s\nfrom :%d\n\n", name,
>> get_object_mark(object));
>
> FWIW, this line is now too long (exceeds 80 columns). Good catch on
> the casting though.
>
> -John
>
> PS Sorry for the duplicate Felipe... I still need to get used to
> hitting "Reply All". :-)
The guideline is:
- We try to keep to at most 80 characters per line.
The key word being *try*.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] fast-export: trivial cleanup
On Wed, May 8, 2013 at 9:16 PM, Felipe Contreras
wrote:
> Cast the object to a commit, only to get the object back?
>
> Signed-off-by: Felipe Contreras
> ---
> builtin/fast-export.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 8091354..d24b4d9 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -550,7 +550,6 @@ static void get_tags_and_duplicates(struct
> rev_cmdline_info *info,
>
> static void handle_tags_and_duplicates(struct string_list *extra_refs)
> {
> - struct commit *commit;
> int i;
>
> for (i = extra_refs->nr - 1; i >= 0; i--) {
> @@ -562,9 +561,7 @@ static void handle_tags_and_duplicates(struct string_list
> *extra_refs)
> break;
> case OBJ_COMMIT:
> /* create refs pointing to already seen commits */
> - commit = (struct commit *)object;
> - printf("reset %s\nfrom :%d\n\n", name,
> - get_object_mark(&commit->object));
> + printf("reset %s\nfrom :%d\n\n", name,
> get_object_mark(object));
FWIW, this line is now too long (exceeds 80 columns). Good catch on
the casting though.
-John
PS Sorry for the duplicate Felipe... I still need to get used to
hitting "Reply All". :-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
