Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 1 Dec 2018 10:14:44 +0700
From:      Eugene Grosbein <eugen@freebsd.org>
To:        cem@freebsd.org
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r341352 - head/usr.sbin/trim
Message-ID:  <13bec879-3cf5-3f9b-58a3-47f7405e5f77@freebsd.org>
In-Reply-To: <CAG6CVpWMCsoho-gDnuSHWjh=KsbsyL6=xBT%2BQX=dfRhh3Cx-gg@mail.gmail.com>
References:  <201811302157.wAULv2iH057184@repo.freebsd.org> <CAG6CVpWMCsoho-gDnuSHWjh=KsbsyL6=xBT%2BQX=dfRhh3Cx-gg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
01.12.2018 9:34, Conrad Meyer wrote:

> Generally when a committer puts a patch on phabricator, they'll commit
> it when they're ready.

I think this is wrong behaviour of the Phabricator to auto-close a differential
just because some singe commit referred to it. It was not meant to be closed
for now and I've just re-opened the differential.

> In particular this patch hadn't actually been
> approved by any reviewers yet and review discussion was still active.

Is it possible to add hackers@ audience to reviewers list?
I do not see people discussing the topic adding themselves
and it would be wrong to force them.

> As far as I can tell, imp@ hadn't indicated he wasn't going to commit
> his patch himself.

Parts of changes that have consensus were committed. Why is it bad?

> I'd love to be imagining things, but it seems like
> the general trend is you consistently fail to follow basic etiquette
> and consistently ignore review and concerns.
>
> This time around you definitely don't have the excuse of phabricator
> being silent for months — the review hadn't seen any 24h period of
> inactivity since its creation.
> 
> I was tentatively ok with trim staying in based on imp@ taking charge
> of cleaning up the deficiencies, but you've gone around imp@ and
> ignored me completely.

This is untrue: I committed some changes requested by you and imp.

> Please back out trim and consider taking a few days off.  The way trim
> has entered the tree is just not how we do things as a project.  I
> think there's universal consensus that we want a tool that does what
> trim does in base; there's some bikeshedding about whether it should
> live in dd or not (it shouldn't); but rest assured, it will go in in
> some form.  My ask is that it to go in the right way, instead of the
> total mess it has so been so far.

I have not seen universal consensus about backing it out and I disagree
that it is "total mess". The code is small, clean and works.
But if you insist, feel free to remove it.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?13bec879-3cf5-3f9b-58a3-47f7405e5f77>