On Fri, Mar 29, 2019 at 4:32 PM Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
>
> On 03/29, Matheus Tavares Bernardino wrote:
> > On Thu, Mar 28, 2019 at 6:49 PM Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> > > When sending someone elses patch in a slightly modified version, it
> > > may also be useful to add which parts you changed, as it was done in
> > > e8dfcace31 ("poll: use GetTickCount64() to avoid wrap-around issues",
> > > 2018-10-31) for example.
> >
> > Thanks, I didn't know about that! I searched the log and didn't see
> > many of this on patches with 'Helped-by' tags, is there a particular
> > case to use it or not?
>
> Helped-by tags are usually used when you want to give someone credit
> for help you got on a patch that you originally authored. It's up to
> you at which point of involvement you actually want to add the tag, I
> tend to add them whenever someones input significantly
> changes/improves the patch. I think adding it here might be okay,
> it's just less common when sending a patch that someone else authored
> originally.
>
Ok, got it, thanks!
> > > Iirc, the test that is added in this patch does not work on some
> > > platforms, notably MacOS. That would mean that we would break
> > > bisectability at this patch on some platforms if we were to introduce
> > > it here. Therefore I think it would be better to squash this patch
> > > into the next one which fixes these inconsistencies.
> > > Note that I can't test this at the moment, so this concern is only
> > > based on previous discussions that I remember. If that's already
> > > addressed somehow, all the better!
> >
> > Yes, it is already addressed :) The section of these tests that used
> > to break on some platforms is now moved to the next patch which also
> > fixes the platform inconsistencies. Now both patches (this and the
> > next) work on macOS, NetBSD and GNU/Linux.
>
> Great!
>
> > Also every test and job is
> > passing at travis-ci, except by the job named "Documentation"[1]. But,
> > it's weird since these patches don't even touch Documentation/... And
> > master is failing the same job at my fork as well [2]... Any thoughts
> > on that?
>
> Yeah, this error seems to have nothing to do with your patch series.
> Since the last run of travis on master [*1*] at least the asciidoc
> package doesn't seem to have changed, so from a first look I don't
> quite understand what's going on there. In any case, I don't think
> you need to worry about that for now, as it hasn't been triggered by
> your changes (I won't discourage you from looking at why it is failing
> and to try and fix that, but I think your time is probably better
> spent looking at this patch series and the proposal for GSoC for
> now).
>
Ok, thanks again.
> *1*: https://travis-ci.org/git/git/builds/508784487
>
> > [1] https://travis-ci.org/MatheusBernardino/git/builds/512713775
> > [2] https://travis-ci.org/MatheusBernardino/git/builds/513028692
