Date: Fri, 19 Aug 2005 01:32:37 -0700 From: Doug Barton <dougb@FreeBSD.org> To: Colin Percival <cperciva@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/conf newvers.sh Message-ID: <43059925.3090701@FreeBSD.org> In-Reply-To: <200508190356.j7J3uj5D095435@repoman.freebsd.org> References: <200508190356.j7J3uj5D095435@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Colin Percival wrote: > cperciva 2005-08-19 03:56:45 UTC > > FreeBSD src repository > > Modified files: > sys/conf newvers.sh > Log: > Forced commit to note that the preceeding commit also makes it possible > to override the BRANCH specified in newvers.sh via a BRANCH_OVERRIDE > environment variable. This is useful for my FreeBSD Update builds, and > might possibly be useful for someone else at some point. > > Revision Changes Path > 1.72 +0 -0 src/sys/conf/newvers.sh > > http://www.FreeBSD.org/cgi/cvsweb.cgi/src/sys/conf/newvers.sh.diff?&r1=1.71&r2=1.72&f=h I have 3 comments about this commit, none of which are intended to be critical. 1. A "better" way (IMO) to write: if [ "X${BRANCH_OVERRIDE}" != "X" ]; then is case "${BRANCH_OVERRIDE}" in '') ;; *) BRANCH=${BRANCH_OVERRIDE} ;; esac The original reason for writing it this way was to avoid the call to test(1), because case is a shell builtin. This is a style issue, and not a "must have," but I thought I'd mention it. 2. I've had the following in my newvers.sh file for years, it helps me identify exactly when a given kernel was built (which may or may not coincide with the age of the sources): RELEASE="${REVISION}-${BRANCH}-`/bin/date +%m%d`" I like your override idea here, but I'd rather not have to keep track of what the current value of $BRANCH is so that I can include it in my own override variable. What would be more useful to me (and arguably more useful in general, although once again I will not press the point) would be some way to add a string to the BRANCH or RELEASE variables. This would also allow you to simplify the code. I'd have done the following: Index: newvers.sh =================================================================== RCS file: /usr/local/ncvs/src/sys/conf/newvers.sh,v retrieving revision 1.70 diff -u -r1.70 newvers.sh --- newvers.sh 11 Jul 2005 08:34:49 -0000 1.70 +++ newvers.sh 19 Aug 2005 08:25:12 -0000 @@ -32,7 +32,7 @@ TYPE="FreeBSD" REVISION="7.0" -BRANCH="CURRENT" +BRANCH="CURRENT${BRANCH_TAG}" RELEASE="${REVISION}-${BRANCH}" VERSION="${TYPE} ${RELEASE}" This will be a noop when not defined, but then I could do something like this in my build environment: BRANCH_TAG="-`/bin/date +%m%d`" and have it work. You are of course free to ignore all this blathering, it's just a suggestion. :) 3. Even in the hectic environment leading up to a release, I dislike insta-MFCs. I admire your desire to get as much done in as short a time as possible, but everyone makes mistakes, and even a 24 hour period between a commit to HEAD and an MFC can make life easier for everyone in the case of an error, and generally does not "cost" us anything. This is simply an opinion, and a request to think about the topic. It is not a specific objection to this commit. Hope this helps, Doug -- This .signature sanitized for your protection
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?43059925.3090701>