Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 29 Mar 2018 09:33:44 -0700 (PDT)
From:      "Rodney W. Grimes" <freebsd@pdx.rh.CN85.dnsmgr.net>
To:        Ian Lepore <ian@freebsd.org>
Cc:        rgrimes@freebsd.org, Marcelo Araujo <araujo@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   Re: svn commit: r331728 - in stable/11/etc: . rc.d
Message-ID:  <201803291633.w2TGXinX064128@pdx.rh.CN85.dnsmgr.net>
In-Reply-To: <1522340373.49673.112.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
> On Thu, 2018-03-29 at 06:20 -0700, Rodney W. Grimes wrote:
> > > 
> > > Author: araujo
> > > Date: Thu Mar 29 04:51:07 2018
> > > New Revision: 331728
> > > URL: https://svnweb.freebsd.org/changeset/base/331728
> > > 
> > > Log:
> > > ? MFC r329817:
> > I must of missed this when it landed in ^/head
> > 
> > > 
> > > ? The firewall_type is ignored if not set in rc.conf or rc.conf.local,
> > > ? after r190575 there is an option to call rc.firewall with the firewall_type
> > > ? passed in as an argument.
> > > ??
> > > ? Submitted by:	David P. Discher <dpd@dpdtech.com>
> > > ? Sponsored by:	iXsystems Inc.
> > > ? Differential Revision:	https://reviews.freebsd.org/D14286
> > No one accepted it :-(.
> > 
> 
> That's not a blocker for committing; plenty of time elapsed to allow
> anyone to reject the change. IMO, even a flat-out rejection isn't a
> blocker to committing except for things like random or crypto code that
> require formal approval (but I'd certainly think hard about committing
> if people rejected the change, and put some effort into finding a
> compromise first).

It seems that the Phabricator review system is somewhat disfunctional
in that actual review is only happening in some cases.  Some people
have even stated they flat out hate it.  Others say that it is the
way to go.

As araujo@ pointed out he was a "reviewer", but as I'll point out
he didnt accept the review, which causes the landing state to be
wrong, and is kinda implied that anyone commiting a phabricator
change has reviewed it anyway.

The problem is that most people are not notified that a review
of a change is even in process until the commit lands, this is
not a functional communications system.

Requring us all to go sign up like imp@ did to receive all
submitted reviews, imho, is also a non functional situation.

> There were comments added to the review, which makes citing the review
> in the commit useful.

Yes, I have no problem with citing it,
just unhappy no one had bother to accept it.  

> ?I usually also add a note such as '(timed out)'
> after the url, but I've noticed that doing so ruins the automatic
> closing of the review and requires you to manually abandon it instead.

There isnt a way to close it as commited/fixed in rXXXXXX manually?
If not that is yet another shortcoming of phabricator that should
be looked at.

-- 
Rod Grimes                                                 rgrimes@freebsd.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201803291633.w2TGXinX064128>