svn commit: r272752 - releng/10.1/sys/kern

dteske at FreeBSD.org dteske at FreeBSD.org
Thu Oct 9 16:43:43 UTC 2014


Hi Neel,

> -----Original Message-----
> From: Neel Natu [mailto:neelnatu at gmail.com]
> Sent: Thursday, October 9, 2014 9:01 AM
> To: dteske at freebsd.org
> Cc: Neel Natu; src-committers at freebsd.org; svn-src-all at freebsd.org; svn-
> src-releng at freebsd.org
> Subject: Re: svn commit: r272752 - releng/10.1/sys/kern
> 
> Hi Devin,
> 
> On Thu, Oct 9, 2014 at 1:35 AM,  <dteske at freebsd.org> wrote:
> >
> >
> >> -----Original Message-----
> >> From: owner-src-committers at freebsd.org [mailto:owner-src-
> >> committers at freebsd.org] On Behalf Of Neel Natu
> >> Sent: Wednesday, October 8, 2014 10:47 PM
> >> To: dteske at freebsd.org
> >> Cc: Neel Natu; src-committers at freebsd.org; svn-src-all at freebsd.org;
> svn-
> >> src-releng at freebsd.org
> >> Subject: Re: svn commit: r272752 - releng/10.1/sys/kern
> >>
> >> Hi Devin,
> >>
> >> On Wed, Oct 8, 2014 at 9:53 PM,  <dteske at freebsd.org> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: owner-src-committers at freebsd.org [mailto:owner-src-
> >> >> committers at freebsd.org] On Behalf Of Neel Natu
> >> >> Sent: Wednesday, October 8, 2014 8:39 AM
> >> >> To: src-committers at freebsd.org; svn-src-all at freebsd.org; svn-src-
> >> >> releng at freebsd.org
> >> >> Subject: svn commit: r272752 - releng/10.1/sys/kern
> >> >>
> >> >> Author: neel
> >> >> Date: Wed Oct  8 15:39:24 2014
> >> >> New Revision: 272752
> >> >> URL: https://svnweb.freebsd.org/changeset/base/272752
> >> >>
> >> >> Log:
> >> >>   MFC r272270:
> >> >
> >> > I hate to pick nits, but I believe this revision (272752 in releng/10.1)
> >> > should (I suggest; deferring to re@ for final prognosis) perhaps have
> >> > _not_ been an MFC from head (r272270; as was performed) but
> >> > perhaps have instead been MFS from stable/10 (r272726).
> >> >
> >>
> >> The svn command used to do the merge was:
> >> # svn merge -c 272726 ^/stable/10 releng/10.1
> >>
> >> This is exactly as per the SVN merge guidelines into releng as documented
> >> here:
> >> https://www.freebsd.org/doc/en/articles/committers-guide/subversion-
> >> primer.html
> >>
> >
> > Hi Neel,
> >
> > Nowhere in that document does it describe "MFS".
> > I'm happy you did the merge as it was supposed to be done.
> > However, your commit message is inaccurate.
> >
> 
> I don't agree - the commit message is entirely self-consistent but I
> see why you might think otherwise.
> 
> FWIW here is an analysis of all commits into releng/10.1.
> 
> There are six commits with a commit message of "MFC <head_revision>":
> https://svnweb.freebsd.org/changeset/base/272684
> https://svnweb.freebsd.org/changeset/base/272669
> https://svnweb.freebsd.org/changeset/base/272612
> https://svnweb.freebsd.org/changeset/base/272611
> https://svnweb.freebsd.org/changeset/base/272608
> https://svnweb.freebsd.org/changeset/base/272752
> 
> There is one commit with a commit message of "MFC <stable_10_revision>":
> https://svnweb.freebsd.org/changeset/base/272682
> 
> There is one commit with a commit message of "MF10
> <stable_10_revision>":
> https://svnweb.freebsd.org/changeset/base/272819
> 
> Clearly there are different styles used by developers but the numbers
> suggest that "MFC <head_revision>" is the preferred one.
> 
> best
> Neel
> 
[Devin Teske] 

Thank you for the recent sampling. I was extrapolating
more from a longer experience encompassing multiple
releases, and -- thanks to your recent poll -- it looks like
what I have often deemed "the better acronyms" are
falling out-of-use. ;(

It would appear as though commits into lower branches
are describing the intent of the commit, not the letter of
the commit. Or in other words, I've always tried to make
my commit messages (because I've found others whom
have done-so have a laudable history with the project)
reflect the action of the commit (MFS if merging from
stable; MFV if merging from vendor/contrib, etc.) rather
than reflect the higher-level meaning of the commit
(which will almost always be MFC, except perhaps if say
merging from project space; MFP).

I believe I have observed waxing and waning use of
these acronyms, and your recent poll shows a current
waning trend; however I wish more commits would:

a. describe the individual action in the message rather
than the over-arching, higer-level action
b. Use these acronyms more

If there is strong number of yay's that agree, perhaps
maybe even an addendum or slight modification to the
committer's guide?
-- 
Devin

> >> The commit message used "MFC r272270" because that was the origin of
> >> the change and has the full details about the patch.
> >>
> >
> > I don't care if the subversion primer doesn't talk about "MFS" versus
> > "MFC", but it is technically inaccurate to call your commit an "MFC [of]
> > r272270" because it is in-fact (as you admit) an MFS of r272726 -- which
> > is indeed indicated by mergeinfo.
> >
> > Making someone look up the mergeinfo because the commit message
> > is a inaccurate doesn't make for an efficient/predictable setup.
> > --
> > Cheers,
> > Devin
> >
> > P.S. If I was really on your case, I'd insist that you had put "MFS10 r#"
> > but in all reality, "MFS r#" would have been perfectly acceptable where
> > "MFC r#" is clearly just plain wrong and misleading.
> >
> >> best
> >> Neel
> >>
> >> > The nit being that mergeinfo now shows (unnaturally) that things
> >> > flowed from head -> stable / head -> releng versus
> >> > head -> stable -> releng as I suggest would have been cleaner for
> >> > historical analysis.
> >> > --
> >> > Cheers,
> >> > Devin
> >> >
> >> >>
> >> >>   tty_rel_free() can be called more than once for the same tty so make
> >> sure
> >> >>   that the tty is dequeued from 'tty_list' only the first time.
> >> >>
> >> >>   Approved by:        re (glebius)
> >> >>
> >> >> Modified:
> >> >>   releng/10.1/sys/kern/tty.c
> >> >> Directory Properties:
> >> >>   releng/10.1/   (props changed)
> >> >>
> >> >> Modified: releng/10.1/sys/kern/tty.c
> >> >>
> >>
> ==========================================================
> >> >> ====================
> >> >> --- releng/10.1/sys/kern/tty.c        Wed Oct  8 15:30:59 2014
> (r272751)
> >> >> +++ releng/10.1/sys/kern/tty.c        Wed Oct  8 15:39:24 2014
> (r272752)
> >> >> @@ -1055,13 +1055,13 @@ tty_rel_free(struct tty *tp)
> >> >>       tp->t_dev = NULL;
> >> >>       tty_unlock(tp);
> >> >>
> >> >> -     sx_xlock(&tty_list_sx);
> >> >> -     TAILQ_REMOVE(&tty_list, tp, t_list);
> >> >> -     tty_list_count--;
> >> >> -     sx_xunlock(&tty_list_sx);
> >> >> -
> >> >> -     if (dev != NULL)
> >> >> +     if (dev != NULL) {
> >> >> +             sx_xlock(&tty_list_sx);
> >> >> +             TAILQ_REMOVE(&tty_list, tp, t_list);
> >> >> +             tty_list_count--;
> >> >> +             sx_xunlock(&tty_list_sx);
> >> >>               destroy_dev_sched_cb(dev, tty_dealloc, tp);
> >> >> +     }
> >> >>  }
> >> >>
> >> >>  void
> >> >
> >> >
> >
> >



More information about the svn-src-all mailing list