Re: [PATCH v6b 5/8] branch: drop non-commit error reporting

[Matthieu Moy]

Junio C Hamano  writes:

> While I was trying to address your "actually already report", I
> spotted a typo and then found that the early part of the second
> paragraph is a bit hard, so here is what I ended up with.

LGTM.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 v6b 5/8] branch: drop non-commit error reporting

[Junio C Hamano]

Matthieu Moy  writes:

> Karthik Nayak  writes:
>
>> Remove the error "branch '%s' does not point at a commit" in
>> apppend_ref() which reports branch refs which do not point to
>> commits. Also remove the error "some refs could not be read" in
>> print_ref_list() which is triggered as a consequence of the first
>> error.
>>
>> This seems to be the wrong codepath whose purpose is not to diagnose
>> and report a repository corruption. If we care about such a repository
>> corruption, we should report it from fsck instead.
>
> (We actually already report it from fsck indeed)
>
>> This also helps in a smooth port of branch.c to use ref-filter APIs
>> over the following patches. On the other hand, ref-filter ignores refs
>> which do not point at commits silently.
>
> Seems much better. Thanks,

Yes, it seems that I can just replace 5/8 with this and the
remainder can stay as they are.

While I was trying to address your "actually already report", I
spotted a typo and then found that the early part of the second
paragraph is a bit hard, so here is what I ended up with.

branch: drop non-commit error reporting

Remove the error "branch '%s' does not point at a commit" in
append_ref(), which reports branch refs which do not point to
commits.  Also remove the error "some refs could not be read" in
print_ref_list() which is triggered as a consequence of the first
error.

The purpose of these codepaths is not to diagnose and report a
repository corruption.  If we care about such a corruption, we
should report it from fsck instead, which we already do.

This also helps in a smooth port of branch.c to use ref-filter APIs
over the following patches. On the other hand, ref-filter ignores refs
which do not point at commits silently.

Based-on-patch-by: Jeff King 
Helped-by: Junio C Hamano 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
Signed-off-by: Junio C Hamano 

Thanks.
--
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 v6b 5/8] branch: drop non-commit error reporting

[Karthik Nayak]

On Fri, Sep 25, 2015 at 9:30 PM, Junio C Hamano  wrote:
> Matthieu Moy  writes:
>
>> Karthik Nayak  writes:
>>
>>> Remove the error "branch '%s' does not point at a commit" in
>>> apppend_ref() which reports branch refs which do not point to
>>> commits. Also remove the error "some refs could not be read" in
>>> print_ref_list() which is triggered as a consequence of the first
>>> error.
>>>
>>> This seems to be the wrong codepath whose purpose is not to diagnose
>>> and report a repository corruption. If we care about such a repository
>>> corruption, we should report it from fsck instead.
>>
>> (We actually already report it from fsck indeed)
>>
>>> This also helps in a smooth port of branch.c to use ref-filter APIs
>>> over the following patches. On the other hand, ref-filter ignores refs
>>> which do not point at commits silently.
>>
>> Seems much better. Thanks,
>
> Yes, it seems that I can just replace 5/8 with this and the
> remainder can stay as they are.
>
> While I was trying to address your "actually already report", I
> spotted a typo and then found that the early part of the second
> paragraph is a bit hard, so here is what I ended up with.
>
> branch: drop non-commit error reporting
>
> Remove the error "branch '%s' does not point at a commit" in
> append_ref(), which reports branch refs which do not point to
> commits.  Also remove the error "some refs could not be read" in
> print_ref_list() which is triggered as a consequence of the first
> error.
>
> The purpose of these codepaths is not to diagnose and report a
> repository corruption.  If we care about such a corruption, we
> should report it from fsck instead, which we already do.
>
> This also helps in a smooth port of branch.c to use ref-filter APIs
> over the following patches. On the other hand, ref-filter ignores refs
> which do not point at commits silently.
>
> Based-on-patch-by: Jeff King 
> Helped-by: Junio C Hamano 
> Mentored-by: Christian Couder 
> Mentored-by: Matthieu Moy 
> Signed-off-by: Karthik Nayak 
> Signed-off-by: Junio C Hamano 
>
> Thanks.

Looks good to me, thanks :)

-- 
Regards,
Karthik Nayak
--
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 v6b 5/8] branch: drop non-commit error reporting

[Matthieu Moy]

Karthik Nayak  writes:

> Remove the error "branch '%s' does not point at a commit" in
> apppend_ref() which reports branch refs which do not point to
> commits. Also remove the error "some refs could not be read" in
> print_ref_list() which is triggered as a consequence of the first
> error.
>
> This seems to be the wrong codepath whose purpose is not to diagnose
> and report a repository corruption. If we care about such a repository
> corruption, we should report it from fsck instead.

(We actually already report it from fsck indeed)

> This also helps in a smooth port of branch.c to use ref-filter APIs
> over the following patches. On the other hand, ref-filter ignores refs
> which do not point at commits silently.

Seems much better. Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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