Federico Simoncelli has posted comments on this change. Change subject: build: closer to open source versioning and release cycle ......................................................................
Patch Set 1: I would prefer that you didn't submit this (5 inline comments) .................................................... Commit Message Line 62: - PACKAGE_RPM_RELEASE - rpm release Line 63: Line 64: Unlike current versioning implementation we disconnect the versioning Line 65: from the git branch information, as the git branch information is aware Line 66: of /past/ versions, while a branch is work toward the /next/ version. Dan was never if favor of this change as he prefers the tagging automation (but I'll leave to him to comment on this as he's the one doing the versioning). At the moment the workflow is: - tag master as version v4.10.0 - ...new patches... - tag master as version v4.11.0 If understood correctly what you're proposing is: - send a patch changing the version to 4.11.0-0.0.master - ...new patches... - send a patch changing the version to 4.11.0-1 - (...new patches?...) - send a patch changing the version to 4.12.0-0.0.master Are there any patches (features/fixes) between 4.11.0-1 and 4.12.0-0.0.master? If not then calling the same release with two different names is confusing. If there are new patches then this resemble the old workflow with the only addition of a staging area (master/alpha/beta) which will be just additional work to maintain (I don't see the benefit). Line 67: Line 68: A valid sequence for release cycle: Line 69: PRODUCT_VERSION=4.11.0 Line 70: PRODUCT_VERSION_SUFFIX=_master Line 91: PACKAGE_RPM_RELEASE=1 Line 92: Line 93: PRODUCT_VERSION=4.12.0 Line 94: PRODUCT_VERSION_SUFFIX=_master Line 95: PACKAGE_RPM_RELEASE=0.0.$(echo PRODUCT_VERSION_SUFFIX | sed 's/^_//') As a result I just got an rpm named: vdsm-4.11.0-0.0.master.20130227100637.gitc6610d699d7ecd35.el6.x86_64.rpm It looks a little bit extreme to me, "0.0" are downstream matters (anyway I'm not against keeping them here as placeholders), "master" is telling me that nobody has approved this as an alpha/beta yet (but I'm not 100% sure as we go back to master between alpha and beta, seeing your workflow above), the date (which is also embedded in the rpm) and a 16 git hash. Beside the git hash I don't see anything extremely relevant for upstream. Line 96: Line 97: Build automation may inject release suffix to the binary package name, an Line 98: example exist in the 'make rpm' target. Line 99: .................................................... File configure.ac Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: Line 21: # Autoconf initialization Line 22: define([PRODUCT_VERSION], [4.11.0]) Just to be more explicit (I already said this commenting the commit message), Dan was never in favor of this and now that we already did the work to extract the versioning from the tags I tend to agree with him. Line 23: define([PRODUCT_VERSION_SUFFIX], [_master]) Line 24: AC_INIT([vdsm], PRODUCT_VERSION[]PRODUCT_VERSION_SUFFIX, [[email protected]]) Line 25: AC_CONFIG_AUX_DIR([build-aux]) Line 26: .................................................... File Makefile.am Line 84: Line 85: srpm: dist Line 86: SUFFIX=".`date -u +%Y%m%d%H%M%S`"; \ Line 87: if [ -d "$(top_srcdir)/.git" ]; then \ Line 88: SUFFIX="$$SUFFIX.git`GIT_DIR="$(top_srcdir)/.git" $(GIT) rev-parse --short=16 HEAD`"; \ Is there a reason to set short to 16? For many projects the default (7) is enough. If you think that SUFFIX (the date) doesn't provide enough uniqueness then let's not use it (the build date is embedded in the rpm anyway). Line 89: fi; \ Line 90: rpmbuild \ Line 91: -ts \ Line 92: --define="release_suffix $$SUFFIX" \ Line 96: rpm: dist Line 97: SUFFIX=".`date -u +%Y%m%d%H%M%S`"; \ Line 98: if [ -d "$(top_srcdir)/.git" ]; then \ Line 99: SUFFIX="$$SUFFIX.git`GIT_DIR="$(top_srcdir)/.git" $(GIT) rev-parse --short=16 HEAD`"; \ Line 100: fi; \ This 4 lines above look duplicated, you can consider to use a global definition or a function. Line 101: rpmbuild \ Line 102: -ta \ Line 103: --define="release_suffix $$SUFFIX" \ Line 104: $(if $(BUILDID),--define="extra_release .$(BUILDID)") \ -- To view, visit http://gerrit.ovirt.org/12448 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a73089d47804ca31e3a4d80aaf811fa011ba1f3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Moran Goldboim <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
