On Fri, Nov 27, 2020 at 03:05:49PM +0100, Andrea Bolognani wrote:
On Fri, 2020-11-27 at 11:28 +0100, Erik Skultety wrote:
> On Fri, Nov 27, 2020 at 10:44:56AM +0100, Andrea Bolognani wrote:
> > On Fri, 2020-11-27 at 09:46 +0100, Erik Skultety wrote:
> > > +++ b/ci/Makefile
> > > -ci-check@%:
> > > - $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
> > > +ci-test@%:
> > > + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS+=test
> >
> > I don't know why this last bit turned from =test in v3 to +=test in
> > v4, but I don't think it should have. Please change it back.
>
> Deliberate decision...the whole point of ci-test is to test, you may want to
> add more flags to ninja but still want to test, we don't document anywhere that
> the flags user sets override our defaults - I've seen user flags to be appended
> to global env variables in Makefiles so I did the same here. If you want me to
> change it back without documenting that when user selects a CI target expecting
> something to happen turning out completely different because they set a certain
> variable according to the help, I can sure do that (I don't care that much),
> but it's IMO confusing and this way it's more foolproof.
Since you're just converting from autotools to meson in this commit,
the underlying behavior should probably not change, so I'd leave it
as it is regardless of whether or not you consider the change to be
for the better.
ci-test was always intended to be a convenient shorthand anyway, the
idea being that you'd use it for the basic case and then switch to
ci-build CI_NINJA_ARGS=... as soon as you started to outgrow it.
Fair enough...I changed it back and pushed.
Thanks,
Erik