Patch Set 3: Code-Review-1 (2 comments)
(sorry, forgot to submit these comments, lying around as drafts for some hours) https://gerrit.osmocom.org/#/c/3973/3/Makefile.am File Makefile.am: Line 12: .PHONY: release this should be directly above the definition of the target, i.e. in this case in the osmo-release.mk, in this way: .PHONY: release release: ... One advantage of having the .PHONY in the .mk file is that you don't need to repeat the .PHONY in every Makefile.am using the .mk file. https://gerrit.osmocom.org/#/c/3973/3/osmo-release.mk File osmo-release.mk: Line 6: NEW_VER := $(shell bumpversion --list --current-version $(VERSION) $(REL) --allow-dirty 2>/dev/null | awk -F '=' '{ print $$2 }') This would still attempt to invoke 'bumpversion' if it is not there. If it is there, and if bumpversion had any error messages we need to see to know what went wrong, we would now silence those. Is it somehow possible to only ever call this after we actually invoked the 'make release'? I think make doesn't allow evaluating a variable within a make target... But we could also do something like NEW_VER := $(shell test -z "$(BUMPVER)" || bumpversion ...) This would not need the 2>/dev/null. Another idea would be to cat the bumpversion result to a file within the release target and then use that. -- To view, visit https://gerrit.osmocom.org/3973 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia6ab79fe2f6b502ea223aa72e929dd3c75f34b5c Gerrit-PatchSet: 3 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: Max <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-HasComments: Yes
