Patch Set 4:

(9 comments)

https://gerrit.osmocom.org/#/c/3132/4//COMMIT_MSG
Commit Message:

Line 9: * reorder builds to avoid rm -rf invocation
why?


Line 10: * avoid useless double autoreconf
the autoreconf is not useless, it only becomes so from your dropping 'rm -rf', 
a change with which I actually disagree. I would prefer ensuring a clean slate 
every time.


https://gerrit.osmocom.org/#/c/3132/4/contrib/jenkins-arm.sh
File contrib/jenkins-arm.sh:

Line 6:     $1 --enable-static \
(cosmetic: personally, I'd prefer passing ".." or "." as $1 instead of 
'../configure' and run "$1"/configure)


Line 13: $MAKE $PARALLEL_MAKE \
indenting


Line 18: mkdir -p builddir
removing the rm -rf: have you checked that the jenkins build job running this 
cleans up the workspace? Otherwise there would be built files lying around in 
the workspace and we'd also rebuild over and over in the same build dirs. We 
should make sure it's done from scratch every time, one way or the other.

An alternative if you really dislike 'rm -rf' would be 'git clean -dxf'?


https://gerrit.osmocom.org/#/c/3132/4/contrib/jenkins.sh
File contrib/jenkins.sh:

Line 13:     $1 --enable-static $2 CFLAGS="-Werror" CPPFLAGS="-Werror"
(could just write $ENABLE_SANITIZE instead of passing $2. As before: preference 
of ".." for $1)


Line 14: $MAKE $PARALLEL_MAKE check \
indenting


Line 21: mkdir -p builddir
again make sure we build cleanly every time.


https://gerrit.osmocom.org/#/c/3132/4/contrib/jenkins_common.sh
File contrib/jenkins_common.sh:

Line 8
(seems a lot of trouble to add a _common script for just three lines, but ok)


-- 
To view, visit https://gerrit.osmocom.org/3132
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I24e500e132f5c8e8133d35548cb7b4e4552331d0
Gerrit-PatchSet: 4
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-Reviewer: blobb <[email protected]>
Gerrit-Reviewer: lynxis lazus <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-HasComments: Yes

Reply via email to