Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Update to Theia 0.3.19, build another image from upstream #56

Merged
merged 4 commits into from
Feb 8, 2019

Conversation

evidolob
Copy link
Contributor

@evidolob evidolob commented Feb 7, 2019

Should fix #39

Also include build che-theia-master image from Theia and all extensions master branch.
Removes che-theia-ssh-extension and github-extension which is deprecated and no longer supported(there #31 pull which add ssh as plugin)
Signed-off-by: Yevhen Vydolob yvydolob@redhat.com

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
@evidolob evidolob requested a review from benoitf February 7, 2019 09:14
@@ -39,8 +39,17 @@ RUN if [ ! -z "${GITHUB_TOKEN-}" ]; then \
fi \
fi

#invalidate cashe
ADD https://${GITHUB_TOKEN}:x-oauth-basic@api.github.com/repos/theia-ide/theia/git/refs/heads/master /tmp/master.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it will redo a full build of all remaining layers of theia image 0.3.19 just because master of theia has been updated ?

should the /refs/heads/master be a variable as well (refs/heads/master vs refs/tags/v0.3.19`) so something like
refs/${REF_GITHUB} and with default REF_GITHUB = 'refs/tags/v${THEIA_VERSION}'

BTW I think that travis is not doing any cache of layers as we get a new VM each time so maybe we drop the line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I need to remove that line?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be perfect we could add this line as a macro like we do there:

# Replace macros in Dockerfiles	  # Replace macros in Dockerfiles
  cat ${DIR}/${DOCKERFILE} | sed s/\$\{BUILD_ORGANIZATION\}/${ORGANIZATION}/ | sed s/\$\{BUILD_PREFIX\}/${PREFIX}/ | sed s/\$\{BUILD_TAG\}/${TAG}/ > ${DIR}/.Dockerfile

we could have macros for the checkout tag as well and ref tags.
so Dockerfile for master will have the good line
and Dockerfile for branch will have the good line

as we've macros we can do that easily

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also if you want, let's remove the line and add macro as a follow-up

we could replace as well easily the condition

RUN if [ $THEIA_VERSION == "master" ]; then \
        echo "Cloning Theia master branch"; \
        (git clone --branch master  --single-branch --depth 1 https://github.com/theia-ide/theia ${HOME}/theia-source-code) ; \
    else \
      (git clone --branch v${THEIA_VERSION}  --single-branch --depth 1 https://github.com/theia-ide/theia ${HOME}/theia-source-code); \
    fi

with a single line that will be replaced on the file by build.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macros like
${GIT_BRANCH_NAME} (will be v${THEIA_VERSION} or master)
${GIT_REF} (will be something like refs/tags/v${THEIA_VERSION} or refs/heads/master)

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve by removing the ADD line + follow-up
or use of macros to simplify Dockerfile

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice improvements !

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
@evidolob evidolob merged commit c7e5afd into master Feb 8, 2019
@evidolob evidolob deleted the update-theia branch February 8, 2019 07:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Theia image to the newer version (for example 0.3.19)
2 participants