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

KOGITO-4344 CCE while running Monaco in dev mode #3562

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

treblereel
Copy link
Contributor

@treblereel treblereel commented Feb 9, 2021

@tiagobento
Copy link
Contributor

@karreiro Adding you as a reviewer too. This PR is supposed to fix the Monaco issue when running the webapps with gwt:run. We're removing the properties that disable the checks for testing purposes. Can you please validate that it fixes the Monaco issue?

The other issues (Saving on the testing webapp) are not supposed to be fixed. This PR will be merged along with the fix for that, but it's not ready yet, so this PR will have to wait until the other fix is ready too.

@treblereel
Copy link
Contributor Author

this PR should be merged simultaneously with JSONIX fix

Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @treblereel :-) I confirm this PR fixes the Monaco issue, but removing the flags brings the other issues back:

2021-02-10 15 47 59

@treblereel
Copy link
Contributor Author

treblereel commented Feb 14, 2021

Thank you, @treblereel :-) I confirm this PR fixes the Monaco issue, but removing the flags brings the other issues back:

yes, this is another issue i am trying to fix, that isn't related to monaco ...
in short, any calls to marshalling service (like download the xml) broke the app, CCE

looks like a bug in 2.9.0, i did a simple reproducer, now i am trying to find the fix/walkarounf with gwt-commitee mates

@jomarko jomarko self-requested a review February 17, 2021 08:13
@treblereel
Copy link
Contributor Author

this issue depends on #3570

@sonarcloud
Copy link

sonarcloud bot commented Feb 18, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

64.7% 64.7% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @treblereel! LGTM! However, we need to merge #3582 before merging this one, otherwise, I believe it would break the build :-)

Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @treblereel. ⚠️ Notice: #3582 needs to be merged first.

@jomarko jomarko removed their request for review April 9, 2021 06:18
@jomarko
Copy link

jomarko commented Apr 9, 2021

No resources to review this PR. Proceed without my review.

@domhanak
Copy link
Contributor

@tiagobento @karreiro @treblereel Hello, please advice how to proceed with this PR. It is approved but not merged and has conflicts.

@treblereel
Copy link
Contributor Author

@domhanak hello, yeap, it needs to be rebased, let me do that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants