blanket portmgr approval vs. non-fixing changes (was: svn commit: r417590 - in head/databases/db6: . files and 417595 (revert))

[Marcelo Araujo]

2016-06-27 16:58 GMT+08:00 Matthias Andree <matthias.andree at tu-dortmund.de>:

> Am 27.06.2016 um 10:16 schrieb Mathieu Arnold:
> >
> >
> > +--On 27 juin 2016 16:10:36 +0800 Marcelo Araujo <
> araujobsdport at gmail.com>
> > wrote:
> > | 2016-06-27 16:02 GMT+08:00 Mathieu Arnold <mat at freebsd.org>:
> > |> | Read here for reference:
> > |> |
> > |>
> https://www.freebsd.org/doc/en/books/porters-handbook/makefile-maintainer
> > |> | .html
> > |>
> > |> That pages says, exactly the opposite of what you are trying to says:
> > |>
> > |
> > | No it doesn't!
> > |
> > | And this is the normal workflow:
> > | 1) Port has a maintainer, and it needs update.
> > | 2) Open a PR with the patch.
> > | 3) After 2 weeks, and with timeout; anybody can commit it.
> > |
> > |
> > | And about the ownership and belong to the community, I do believe in
> that,
> > | that is the basic in a legal point of view.
> >
> > That page says that the maintainer has to be consulted, except for
> changes
> > covered by the blanket approval, where the change can be committed
> > immediately.
> >
> > In this case, Sunpoet had every right to commit the thing he did without
> > asking or notifying the maintainer.
>
>
> TL;DR given at the very end.
>
>
> 1. Given the portmgr@ rules, that is our current policy, that portmgr@
> as the overseers of the ports system have delegated, by the blanket
> approval, part of their ultimate responsibility to the public.
>
> 2. What I was meaning to state was that (and I'll not pick at the kind
> soul who has modernized the port) we should only apply the blanket
> approval if ports have fallen into disrepair.
>
>
> 2b. This was not the case with db6, the port wasn't known broken to me,
> so why do we permit and encourage going the fast path for changes that
> do not /repair/ a port (for instance, if it's not building, to fix
> misspellings), and I'm surprised because some two months ago, it has
> already gone through a modernization round after gahr's PR,
> <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208740>, that also
> combined a feature addition and required a bit more work to get right,
> see changesets 415741 and 415743.
>
>
> 3. What would have been bad about filing a PR in this case?
>
> The argument "maintainers aren't doing it" is covered by the maintainer
> timeout.  Anything that does not need the fast path should go through
> some form of review, most naturally through a PR filed to the port's
> maintainer.
>
>
> 4. Do we need to tighten up the set of required tests a committed does
> before committing non-maintainer updates?
>
> I'm scratching my head over this one since the failure in r417590 that
> got remedied in r417595 was rather peculiar, and I'm not sure if anyone,
> including myself, would have figured that out.  It might have slipped
> through the cracks even if I'd reviewed it.
>
> 4b. It's probably better to extend the committer's guide and/or porter's
> handbook and have a list of test recommendations where we list things
> that trigger a certain test requirement.  I. e. things to test IN
> ADDITION to the usual "poudriere testport" or "make DEVELOPER=yes clean
> all check-plist package" and portlint coverage.  Meaning that if someone
> tweaks any of the WRK* and *DIR/*SRC-related variables, "also test 'make
> clean extract do-patch makepatch' on a copy of the port directory" or
> thereabouts.
>

mat@ thanks for all the explanation and time.

Unfortunately, I still make things a bit manual at my side, but usually my
testbed is:
1) Portlint.
2) Make and likes on i386 and amd64(clean vm).

I think, include more information about test recommendations is always good.


>
>
> There seem to be at least two distinct camps, in one camp, maintainers
> go along Marcelo's and my trains of thought, in the other, maintainers
> cherish fast and low-ceremony progress, marino has argued along these
> lines, and some other portmgr@ members have pushed for progress in the
> past.
>
> I don't mean to bikeshed or split up our project here, just refine our
> existing policies.
>

>
> TL;DR:  I propose:
>
> - the blanket approval should be tightened up a bit and encourage that
> non-trivial and non-urgent changes go through the PR and invoke
> mantainer timeout after the shortest possible period.
>

Personally, I like the first option! And in addition, we have phabricator
as an option too, at least for src, the reviews are made very quickly.
So, could be defined a short timeout, at least for those that are active
and would like to help make a review, seems something reasonable.

Also I do understand about all the modernization and definitely we need it,
maybe 2 days timeout is enough for an active maintainer to reply that he is
busy or he is working on that.



>
> - we discuss about an assisting set of "change these variables
> foo.*regexp, and you also need to test 'make foo' and 'make bar'" rules
> in the form of a concise list.
>
>

Thanks,
-- 

-- 
Marcelo Araujo            (__)araujo at FreeBSD.org
\\\'',)http://www.FreeBSD.org <http://www.freebsd.org/>   \/  \ ^
Power To Server.         .\. /_)