From owner-svn-src-all@FreeBSD.ORG Sun Oct 13 08:14:35 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 2F471BB4 for ; Sun, 13 Oct 2013 08:14:35 +0000 (UTC) (envelope-from nwhitehorn@anacreon.physics.wisc.edu) Received: from anacreon.physics.wisc.edu (unknown [IPv6:2607:f388:101c:0:216:cbff:fe39:3fae]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 02F9F2851 for ; Sun, 13 Oct 2013 08:14:34 +0000 (UTC) Received: from anacreon.physics.wisc.edu (localhost [127.0.0.1]) by anacreon.physics.wisc.edu (8.14.7/8.14.7) with ESMTP id r9D8EXWM034030; Sun, 13 Oct 2013 03:14:33 -0500 (CDT) (envelope-from nwhitehorn@anacreon.physics.wisc.edu) Received: from localhost (nwhitehorn@localhost) by anacreon.physics.wisc.edu (8.14.7/8.14.7/Submit) with ESMTP id r9D8EW5X034027; Sun, 13 Oct 2013 03:14:32 -0500 (CDT) (envelope-from nwhitehorn@anacreon.physics.wisc.edu) Date: Sun, 13 Oct 2013 03:14:32 -0500 (CDT) From: Nathan Whitehorn To: Devin Teske Subject: Re: svn commit: r256343 - in head/usr.sbin/bsdinstall: . scripts In-Reply-To: <13CA24D6AB415D428143D44749F57D720FC5B8F1@LTCFISWMSGMB21.FNFIS.com> Message-ID: References: <201310112041.r9BKfZeT002056@svn.freebsd.org> <5258F9B3.7030101@freebsd.org> <13CA24D6AB415D428143D44749F57D720FC5B547@LTCFISWMSGMB21.FNFIS.com> <86txgmr0oh.fsf@nine.des.no> <13CA24D6AB415D428143D44749F57D720FC5B8F1@LTCFISWMSGMB21.FNFIS.com> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; format=flowed; charset=US-ASCII Cc: "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "Teske, Devin" , Nathan Whitehorn , "svn-src-head@freebsd.org" , =?ISO-8859-15?Q?Dag-Erling_Sm=F8rgrav?= X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 13 Oct 2013 08:14:35 -0000 On Sat, 12 Oct 2013, Teske, Devin wrote: > > On Oct 12, 2013, at 8:03 AM, Dag-Erling Sm?rgrav wrote: > >> "Teske, Devin" writes: >>> The code to replace the use of camcontrol is a a *very* complex parsing >>> of the geom XML configuration data stashed in sysctl. jmg@ started the >>> ball rolling on that. >> >> You realize there is a text version as well? >> >>> Yes. Which has been discussed at-length, you didn't need to put a >>> sandbag on my back (publicly no less; thanks for that). >> >> Umm, I think Nathan was pretty civil. You're the one who's turning this >> into a catfight. >> > > Reflecting upon the thread to see if you're _right_... > > 1. He stated there were still some issues. [definitely civil] > 2. "I am surprised you committed it especially to stable/10, > before those issues were resolved." [civil? or inflammatory?] > 3. "I'm also not sure if people can review their own patches." [misunderstanding] > 4. "Installer regressions are very easy to introduce and very problematic > when created." [statements like that invariably lead people to believe he views > the commit as a regression -- I explained in a follow-up that it is not a regression] > 5. "Real review for installer changes is thus especially important this late in the > release cycle." [I read this invariably as he views that the commit did not go > through "Real review", but again... there is no regression and it's purely value- > add] > 6. "Do you have any plans to fix these issues in the very near future?" [definitely civil] > > What got me ralled up was #'s 2, 4, and 5. Hi Devin -- I'm sorry you felt I was attacking you. I was, as I said, very glad to see someone work on ZFS support in the installer. The patch seems to have only minor issues, most of which were identified earlier. You will note that I am asking you to fix them, but not for a backout, and for you (and everyone else) to ask for review for future non-trivial installer patches, just as you would for changes to any discrete unit of the operating system. With respect to the patch itself, there are a few other architectural things that need to be fixed: the man page needs updating, you did not update the part of the installer that does unattended installations, etc. The end comment was not to say that this patch *is* a regression -- it clearly isn't since it adds a new feature and doesn't touch existing ones -- but a general comment that the installer is sensitive to them. Unlike most parts of the system, it is run only rarely (developers updating their systems from SVN don't reinstall them from media) and is simultaneously the first thing and, until recently, the *only* thing, new users see when they first download FreeBSD. So bugs can easily not be found but, when they are found, totally cripple all other features of the operating system since it can't be installed for anything else to even be used. Similar things can be true for things that are not quite regressions (new features that don't work as advertised, typos in menus) since the chance we notice them is very low while the chance new users do is high. This is why the review requirements, and testing times, for installer changes are typically quite high. This is especially true in the run-up to a release. -Nathan