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

chore: faster build of docker images #1279

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

vitaliy-guliy
Copy link
Contributor

Signed-off-by: Vitaliy Gulyy [email protected]

What does this PR do?

Removes pre-building of Che-Theia before building of docker images.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

No issue

How to test this PR?

  • clone che-theia repository
  • switch to this branch
  • build images with build.sh

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Happy Path Channel

HAPPY_PATH_CHANNEL=stable

@che-bot
Copy link
Contributor

che-bot commented Dec 21, 2021

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia quay.io/crw_pr/che-theia:1279
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1279

Test product:

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

@codecov
Copy link

codecov bot commented Dec 21, 2021

Codecov Report

Merging #1279 (8d933d5) into main (c299f59) will increase coverage by 3.57%.
The diff coverage is 44.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1279      +/-   ##
==========================================
+ Coverage   32.78%   36.36%   +3.57%     
==========================================
  Files         290      325      +35     
  Lines        9885    10574     +689     
  Branches     1457     1367      -90     
==========================================
+ Hits         3241     3845     +604     
- Misses       6641     6724      +83     
- Partials        3        5       +2     
Impacted Files Coverage Δ
...theia-about/src/browser/about-che-theia-dialog.tsx 0.00% <0.00%> (ø)
...credentials/src/browser/che-credentials-service.ts 0.00% <0.00%> (ø)
...entials/src/browser/credentials-frontend-module.ts 0.00% <0.00%> (ø)
...eia-credentials/src/common/credentials-protocol.ts 0.00% <0.00%> (ø)
...eia-credentials/src/node/che-credentials-server.ts 0.00% <0.00%> (ø)
...s/src/node/che-theia-credentials-backend-module.ts 0.00% <0.00%> (ø)
...ashboard/src/browser/che-theia-dashboard-module.ts 0.00% <0.00%> (ø)
...ia-dashboard/src/browser/theia-dashboard-client.ts 0.00% <0.00%> (ø)
...rowser/src/browser/che-mini-browser-environment.ts 0.00% <0.00%> (ø)
...in-ext/src/browser/che-sidecar-file-system-main.ts 100.00% <ø> (ø)
... and 290 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6538c23...8d933d5. Read the comment docs.

@@ -13,7 +13,6 @@ set -e
set -o pipefail

parse "$@"
yarn ${YARN_OPTS}
Copy link
Member

Choose a reason for hiding this comment

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

Building it with yarn allows failing faster when there's a compilation error. So, not to wait while it fails within a docker build.

Copy link
Member

Choose a reason for hiding this comment

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

On the other hand, as we have a dedicated GH action for validating sources build, it's a good idea to get rid of it.

Copy link
Member

Choose a reason for hiding this comment

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

Please, take care of the --root-yarn-opts parameter as well. Otherwise, we'll have the dead code.
See the details in the PR https://github.com/eclipse-che/che-theia/pull/725/files
Also, need to check ./build.sh --root-yarn-opts:--ignore-scripts calls.

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.

3 participants