svn commit: r341352 - head/usr.sbin/trim

[Eugene Grosbein]

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.