search

thaliproject / CI

CI project for testing mobile devices
MIT License
2 stars 3 forks source link

implement CI doesn't replace all substitution variables for it's scripts #85

Closed larryonoff closed 7 years ago

larryonoff commented 7 years ago

This change is Reviewable

larryonoff commented 7 years ago

please review

czyzm commented 7 years ago

Reviewed 1 of 2 files at r1. Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.

builder/virtual.js, line 93 at r1 (raw file):

    TARGET_BRANCH: job.targetBranch,
    BUILD_SCRIPT_PATH: scr,
    BUILD_SCRIPT: job.test ? scr + ' true' : scr,

Is this 'true' param used somehow?

Comments from Reviewable

larryonoff commented 7 years ago

Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.

_builder/virtual.js, line 93 at r1 (raw file):_

Previously, czyzm wrote… > Is this 'true' param used somehow? >

This's a good question. I assume it was introduced for CI test mode by @mlesnic

Comments from Reviewable

czyzm commented 7 years ago

Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.

builder/virtual.js, line 93 at r1 (raw file):

Previously, larryonoff (larryonoff) wrote… > This's a good question. I assume it was introduced for CI test mode by @mlesnic >

Look at the comment in #83 review - I think it should be removed.

Comments from Reviewable

czyzm commented 7 years ago

Reviewed 1 of 2 files at r1. Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

yaronyg commented 7 years ago
:lgtm:

Reviewed 2 of 2 files at r1. Review status: all files reviewed at latest revision, 1 unresolved discussion.

_Comments from Reviewable_

yaronyg commented 7 years ago

But my LGTM isn't worth as much as Rockwell's so please go with their approval.

lesn1kk commented 7 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion.

builder/virtual.js, line 93 at r1 (raw file):

Previously, czyzm wrote… > Look at the comment in #83 review - I think it should be removed. >

Please remove it.

Comments from Reviewable

larryonoff commented 7 years ago

Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.

builder/virtual.js, line 93 at r1 (raw file):

Previously, mlesnic (Marcin Lesniczek) wrote… > Please remove it. >

Done.

Comments from Reviewable

larryonoff commented 7 years ago

@mlesnic @czyzm please review updated PR

czyzm commented 7 years ago
:lgtm:

Reviewed 1 of 1 files at r2. Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable