Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Devfile Editor #254

Merged
merged 3 commits into from
May 27, 2021
Merged

Fix Devfile Editor #254

merged 3 commits into from
May 27, 2021

Conversation

olexii4
Copy link
Contributor

@olexii4 olexii4 commented May 27, 2021

Signed-off-by: Oleksii Orel oorel@redhat.com

What does this PR do?

Fix DevfileEditor onChange callback.

What issues does this PR fix or reference?

Fixes eclipse-che/che#19873

Signed-off-by: Oleksii Orel <oorel@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented May 27, 2021

✅ E2E dashboard tests succeed 🎉

See Details

Tested with Eclipse Che on K8S (minikube v1.1.1)

  • Use comment "[dashboard-ci-test]" to rerun the dashboard e2e tests

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

@che-bot
Copy link
Contributor

che-bot commented May 27, 2021

✅ E2E dashboard tests succeed 🎉

See Details

Tested with Eclipse Che on K8S (minikube v1.1.1)

  • Use comment "[dashboard-ci-test]" to rerun the dashboard e2e tests

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

@akurinnoy
Copy link
Contributor

@olexii4 please fix the unit test for the devfile editor.

} catch (e) {
console.error('Devfile parse error', e);
return;
}
if (this.areEqual(this.props.workspace.devfile as che.WorkspaceDevfile, devfile)) {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if now we should start comparing string value somehow, to allow users cancel their empty space changes, like my:
Screenshot_20210527_163938
I've created a separate issue for that eclipse-che/che#19882

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

It works fine in case of invalid yaml, so it fully resolves the referenced issue.
I found one more place to improve, but it's a bit different story, so created another issue.

@codecov
Copy link

codecov bot commented May 27, 2021

Codecov Report

Merging #254 (b634287) into main (2a98936) will decrease coverage by 0.00%.
The diff coverage is 30.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
- Coverage   49.95%   49.94%   -0.01%     
==========================================
  Files         140      140              
  Lines        5241     5246       +5     
  Branches      846      846              
==========================================
+ Hits         2618     2620       +2     
- Misses       2382     2385       +3     
  Partials      241      241              
Impacted Files Coverage Δ
src/components/DevfileEditor/index.tsx 28.42% <0.00%> (+0.21%) ⬆️
src/pages/WorkspaceDetails/EditorTab/index.tsx 0.00% <0.00%> (ø)
src/pages/GetStarted/CustomWorkspaceTab/index.tsx 58.42% <66.66%> (+0.09%) ⬆️

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 2a98936...b634287. Read the comment docs.

@github-actions
Copy link

Docker image build succeeded: docker.io/maxura/che-dashboard:che-dashboard-pull-254

@che-bot
Copy link
Contributor

che-bot commented May 27, 2021

✅ E2E dashboard tests succeed 🎉

See Details

Tested with Eclipse Che on K8S (minikube v1.1.1)

  • Use comment "[dashboard-ci-test]" to rerun the dashboard e2e tests

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

@olexii4 olexii4 merged commit ed3c194 into main May 27, 2021
@olexii4 olexii4 deleted the CRW-1717 branch May 27, 2021 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard behaves like there are no changes if Devfile editor has invalid yaml
5 participants