On 05/02, Brandon Williams wrote:
> On 05/02, Stefan Beller wrote:
> > On Tue, May 2, 2017 at 10:25 AM, Brandon Williams <bmw...@google.com> wrote:
> > > On 05/01, Stefan Beller wrote:
> > >> On Mon, May 1, 2017 at 6:02 PM, Brandon Williams <bmw...@google.com>
> > >> 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
>From our offline discussion --quiet doesn't do what we want here. We
are checking (1) if the commit exists and (2) if it is reachable. Using
--quiet would only satisfy (1)
>
> > >
> > > 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
--
Brandon Williams
