This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Patch Set 4: Code-Review-1
(4 comments)
I'd love to +2 to move ahead, but I find it really hard to follow what the code is doing. Half the bash functions called are assumed to be defined somewhere else, while the link from the commit log shows me that those again should call back into bash functions defined here. This needs to be made simpler, or it needs very thorough in-code documentation defining what "users" of these scripts must provide.
Why are these two separate scripts when one sources the other anyway? Will sourcing the other not pose a problem when this one was itself sourced from a different directory?
>From skimming along, I couldn't figure out where the dependencies would actually be built...
Could you collapse the code and/or clarify, please? (Best by in-code comments so that the docs are not "lost" for later readers of the code, and maybe by a more obvious call chain)
I hope it's not because I'm too dense to see the truth in this patch...
General style: we have CamelCase for python/C++ classes, but that's about it. Otherwise all code uses underscored_names AFAICT. I find it too Java-like to have LongFunctionNamesInCamelCase :) ... but YMMV, I would still accept the patch despite the naming style.
https://gerrit.osmocom.org/#/c/2465/4/scripts/osmo-artifacts.sh
File scripts/osmo-artifacts.sh:
Line 5: genericDeps "getBranchAndRevByLocalRepo"
where is genericDeps defined?
Ah, it comes from that "external" script https://github.com/blobbsen/diy-artifacts/blob/master/jenkins-openBSC.sh
Line 61: generateArtifactHashes() {
so this function writes various checksums to stdout -- are these just for the log or actually used somewhere?
Line 70: sleep 1
why sleep?
https://gerrit.osmocom.org/#/c/2465/4/scripts/osmo-build.sh
File scripts/osmo-build.sh:
Line 62: buildProject
where is buildProject defined? -- also in the "external" script
--
To view, visit https://gerrit.osmocom.org/2465
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifee0a2f837d23b19aa5326f810234d5452e47484
Gerrit-PatchSet: 4
Gerrit-Project: osmo-ci
Gerrit-Branch: master
Gerrit-Owner: blobb <dr.blobb at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes