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

chore: Align to upstream changes #1142

Merged
merged 5 commits into from
Jun 17, 2021
Merged

Conversation

RomanNikitenko
Copy link
Member

@RomanNikitenko RomanNikitenko commented Jun 16, 2021

What does this PR do?

There were breaking changes for che-theia in upstream.
The PR aligns che-theia to upstream changes:

  • adapt che-theia to the changes eclipse-theia/theia@aa4fe4a
  • adapt che-theia to eclipse-theia/theia@f9c0311
  • Install 'libsecret' as it's used by keytar that Theia depends on
  • Update git built-in to the version that supports FileDecoration API
  • Align Task Service to upstream changes: eclipse-theia/theia@d563b6b
  • Update yarn.lock. As result some versions were updated.
    For example, @types/prettier from 2.2.3 to 2.3.0. Formatting changes in the PR is related to this update.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#19866
eclipse-che/che#19984

How to test this PR?

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

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
@che-bot

This comment has been minimized.

@codecov
Copy link

codecov bot commented Jun 16, 2021

Codecov Report

Merging #1142 (ca2038e) into master (eeb4526) will increase coverage by 3.24%.
The diff coverage is 55.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1142      +/-   ##
==========================================
+ Coverage   29.45%   32.69%   +3.24%     
==========================================
  Files         277      290      +13     
  Lines        9336     9872     +536     
  Branches     1380     1515     +135     
==========================================
+ Hits         2750     3228     +478     
- Misses       6487     6544      +57     
- Partials       99      100       +1     
Impacted Files Coverage Δ
...theia-about/src/browser/about-che-theia-dialog.tsx 0.00% <ø> (ø)
...e-sync-tracker/src/browser/sync-process-tracker.ts 0.00% <0.00%> (ø)
...ovisioner/src/node/git-configuration-controller.ts 0.00% <0.00%> (ø)
...in-ext/src/browser/che-sidecar-file-system-main.ts 100.00% <ø> (ø)
...eia-plugin-remote/src/node/hosted-plugin-remote.ts 0.00% <0.00%> (ø)
...theia-plugin-remote/src/node/plugin-host-custom.ts 0.00% <0.00%> (ø)
...in-remote/src/node/plugin-remote-backend-module.ts 0.00% <0.00%> (ø)
...theia-plugin-remote/src/node/plugin-remote-init.ts 0.00% <0.00%> (ø)
...-che-theia-plugin-remote/src/node/plugin-remote.ts 0.00% <0.00%> (ø)
...ugin-remote/src/node/server-plugin-proxy-runner.ts 0.00% <0.00%> (ø)
... and 101 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 340363a...ca2038e. Read the comment docs.

Signed-off-by: Artem Zatsarynnyi <azatsary@redhat.com>
@azatsarynnyy
Copy link
Member

let's check with the libsecret installed

@che-bot
Copy link
Contributor

che-bot commented Jun 16, 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:1142
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1142

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

Signed-off-by: Artem Zatsarynnyi <azatsary@redhat.com>
@azatsarynnyy
Copy link
Member

Now, git built-in fails:
image

trying to update git built-in to 1.52.1 that should support FileDecoration API eclipse-theia/theia#8911

@che-bot
Copy link
Contributor

che-bot commented Jun 16, 2021

✅ E2E Happy path tests succeed 🎉

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:1142
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1142

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

@RomanNikitenko
Copy link
Member Author

thank you, Artem, so much for your help and providing additional fixes to align che-theia to upstream changes!

@azatsarynnyy
Copy link
Member

I'm checking the UBI image manually to verify that adding libsecret solves the start-up issue.

@benoitf
Copy link
Contributor

benoitf commented Jun 17, 2021

@azatsarynnyy it should be covered by
Happy Path / happy-path (ubi8, next) (pull_request) Successful in 52m

@azatsarynnyy
Copy link
Member

@azatsarynnyy it should be covered by
Happy Path / happy-path (ubi8, next) (pull_request) Successful in 52m

ahh.. sure, I forgot 🤦‍♂️ Thanks!
So, @RomanNikitenko it's ready for review, I believe.

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.

image

tested tasks, git, github, etc and I didn't see issues

@benoitf
Copy link
Contributor

benoitf commented Jun 17, 2021

@RomanNikitenko 7.32.0 being already tagged I think we're in a good shape to merge ?

@RomanNikitenko
Copy link
Member Author

@RomanNikitenko 7.32.0 being already tagged I think we're in a good shape to merge ?

yes, sure!
I just wanted to give more time for people if someone would like to test/review the changes...

@RomanNikitenko RomanNikitenko merged commit 41877f9 into master Jun 17, 2021
@RomanNikitenko RomanNikitenko deleted the alignToUpstreamChanges branch June 17, 2021 14:15
@che-bot che-bot added this to the 7.33 milestone Jun 17, 2021
@benoitf
Copy link
Contributor

benoitf commented Jun 17, 2021

yes, sure!
I just wanted to give more time for people if someone would like to test/review the changes...

yes, no issue on that (or pressure to get it merge faster)
Just that when it's the start of a sprint I think we're more open to have some little breakages than at the end of the sprint :-)

@zWingz
Copy link
Contributor

zWingz commented Jul 8, 2021

image

tested tasks, git, github, etc and I didn't see issues

How do you test this PR ?
Build a che-theia docker image from PR and use it as theia-editor in devfile ?

@RomanNikitenko
Copy link
Member Author

How do you test this PR ?
Build a che-theia docker image from PR and use it as theia-editor in devfile ?

this is one of the possible options

but actually for testing a PR changes you don't need to build images - you could use Try it buttons
try_it

@RomanNikitenko
Copy link
Member Author

it looks like try it buttons are broken at the moment, but usually it's possible to start a workspace and test changes using the buttons...

@zWingz
Copy link
Contributor

zWingz commented Jul 8, 2021

How do you test this PR ?
Build a che-theia docker image from PR and use it as theia-editor in devfile ?

this is one of the possible options

but actually for testing a PR changes you don't need to build images - you could use Try it buttons
try_it

That is so cool. but it still build a theia-ide@next image ?
Won't this be overwritten by other theia-ide@next images?

@RomanNikitenko
Copy link
Member Author

RomanNikitenko commented Jul 8, 2021

Won't this be overwritten by other theia-ide@next images?

You can go to Happy path tests DevFile => the corresponding meta.yaml file to see what images are used for the current PR.
There is a unique version of che-theia image...

Screenshot 2021-07-08 at 11 29 57

@zWingz
Copy link
Contributor

zWingz commented Jul 8, 2021

@RomanNikitenko thanks

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.

5 participants