Change in osmo-ci[master]: gerrit-verifications/master-builds: with_manuals

[osmith]

osmith has posted comments on this change. ( https://gerrit.osmocom.org/12045 )

Change subject: gerrit-verifications/master-builds: with_manuals
......................................................................


Patch Set 2:

(4 comments)

https://gerrit.osmocom.org/#/c/12045/2/jobs/gerrit-verifications.yml
File jobs/gerrit-verifications.yml:

https://gerrit.osmocom.org/#/c/12045/2/jobs/gerrit-verifications.yml@97
PS2, Line 97:             (FIRMWARE_VERSION == "femtobts_v2.7" && BTS_MODEL == "sysmo" && with_manuals == True) ||
> why building thm here instead of "master" version?
I had this first:
  (FIRMWARE_VERSION == "master" && with_manuals == True)

But then I've realized that the missing BTS_MODEL means: build this with FIRMWARE_VERSION == "master" for *every* BTS_MODEL. Here's a nice visualization:

https://jenkins.osmocom.org/jenkins/view/master/job/master-osmo-bts/

I've moved it to this line, so the manuals really only get built once.


https://gerrit.osmocom.org/#/c/12045/2/jobs/gerrit-verifications.yml@177
PS2, Line 177:             (with_vty == "False" && with_dsp == "none" && FIRMWARE_VERSION=="master" && with_manuals == False) ||
> I'd also enable it here instead of the sysmo one.
Ack


https://gerrit.osmocom.org/#/c/12045/2/jobs/gerrit-verifications.yml@190
PS2, Line 190:             (IU == "--enable-iu" && with_manuals == True) ||
> Since enable iu takes more time, probably makes sense to enable document build in disable-iu instead […]
Ack


https://gerrit.osmocom.org/#/c/12045/2/jobs/master-builds.yml
File jobs/master-builds.yml:

https://gerrit.osmocom.org/#/c/12045/2/jobs/master-builds.yml@206
PS2, Line 206:           cmd: GTP="$GTP" ./contrib/jenkins.sh --publish
> Shouldn't publish be called conditionally based on with_manuals? see GTP="$GTP" as example. […]
I think the
  GTP="$GPT"
is redundant here, the variable should automatically be set as environment variable. But I've missed adjusting the docker lines, they will need to get the new with_manuals environment variable passed through. I'll fix this.

I'd rather use with_manuals than MANUALS, so we could extend it with building man pages later (see commit message).

To answer your question, I don't see a nice way of implementing this. We could do it like that, but it really gets ugly when we would need to duplicate the whole docker line.

  cmd: |
    if [ "$with_manuals" = "True" ]; then
      ./contrib/jenkins.sh --publish
    else
      ./contrib/jenkins.sh
    fi

Instead I would implement logic in contrib/jenkins.sh, so it only publishes the manuals when both with_manuals is True and --publish is set. For example: https://gerrit.osmocom.org/#/c/osmo-pcu/+/12046/



-- 
To view, visit https://gerrit.osmocom.org/12045
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ci
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0643b17a6be6e86e281b50d78dd7a6858cf99ee
Gerrit-Change-Number: 12045
Gerrit-PatchSet: 2
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Nov 2018 13:37:17 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181130/14ddbeca/attachment.htm>