Thread (27 messages) 27 messages, 2 authors, 2017-05-02

Re: [PATCH v2 5/6] submodule: improve submodule_has_commits

From: Brandon Williams <hidden>
Date: 2017-05-02 19:14:30

On 05/02, Stefan Beller wrote:
On Tue, May 2, 2017 at 10:25 AM, Brandon Williams [off-list ref] wrote:
quoted
On 05/01, Stefan Beller wrote:
quoted
On Mon, May 1, 2017 at 6:02 PM, Brandon Williams [off-list ref] wrote:
quoted
+
+               if (capture_command(&cp, &out, GIT_MAX_HEXSZ + 1) || out.len)
eh, I gave too much and self-contradicting feedback here earlier,
ideally I'd like to review this to be similar as:

    if (capture_command(&cp, &out, GIT_MAX_HEXSZ + 1)
        die("cannot capture git-rev-list in submodule '%s', sub->path);
This wouldn't really work because if you provide a SHA1 to rev-list
which it isn't able to find then it returns a non-zero exit code which
would cause this to die, which isn't the desired behavior.
Oh. In that case, why do we even check for its stdout?
(from rev-lists man page)
       --quiet
           Don’t print anything to standard output. This form is primarily
           meant to allow the caller to test the exit status to see if a range
           of objects is fully connected (or not). It is faster than
           redirecting stdout to /dev/null as the output does not have to be
           formatted.
Sounds good, Let me take a look at using --quiet
quoted
I feel like you're making this a little too complicated, as all I'm
doing is shuffling around already existing logic.  I understand the want
to make things more robust but this seems unnecessarily complex.
ok. I was just giving my thoughts on how I would approach it.

Thanks,
Stefan
-- 
Brandon Williams
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help