On 05/02, Stefan Beller wrote: > On Tue, May 2, 2017 at 10:25 AM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > On 05/01, Stefan Beller wrote: > >> On Mon, May 1, 2017 at 6:02 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > >> > + > >> > + 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 > > > > 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