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.

Sign up for GitHub

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

Jump to bottom

Add model comparison #441

Merged
merged 2 commits into from
Jul 18, 2022
Merged

Add model comparison #441

merged 2 commits into from
Jul 18, 2022

Conversation

sgraband
Copy link
Contributor

@sgraband sgraband commented Apr 4, 2022

Added coffee-comparison extension, that communicates via @eclipsesource/comparison-extension.
Uses git integration to retrieve head file and compare it with current state.
Update node colors to better display the comparison state.
Tree view can be triggered on the coffee file -> Compare with HEAD.
Graphical view can be triggered on the notation file -> Compare with HEAD.

Resolves #438.

@sgraband sgraband requested a review from eneufeld April 4, 2022 09:04
eneufeld
eneufeld requested changes Apr 8, 2022
View reviewed changes
Copy link
Contributor

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

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

Why do we need to add web/coffee-comparison/model/org.eclipse.emfcloud.coffee.model-0.1.0-SNAPSHOT.jar ? Can't we take it from the backend build via a copy?
Please also double check headers of new files

...low.glsp.server/src/org/eclipse/emfcloud/coffee/workflow/glsp/server/gmodel/FlowFactory.java Outdated Show resolved Hide resolved
...low.glsp.server/src/org/eclipse/emfcloud/coffee/workflow/glsp/server/gmodel/NodeFactory.java Outdated Show resolved Hide resolved
web/coffee-comparison/src/browser/coffee-comparison-frontend-configuration.ts Outdated Show resolved Hide resolved
web/coffee-comparison/src/browser/coffee-comparison-frontend-module.ts Outdated Show resolved Hide resolved
web/coffee-comparison/src/browser/custom-label-provider.tsx Outdated Show resolved Hide resolved
web/coffee-comparison/src/node/coffee-comparison-backend-configuration.ts Show resolved Hide resolved
@sgraband
Copy link
Contributor Author

I updated the header eslint config and updated all of the headers. Additionally one of the target platforms locations broke. I changed to location to a (hopefully) more stable release link.

Furthermore i consumed the new comparison-extension version to remove the need of the codicon map and removed the duplicated model from the client.

@sgraband sgraband requested a review from eneufeld April 14, 2022 14:08
eneufeld
eneufeld reviewed Apr 19, 2022
View reviewed changes
Copy link
Contributor

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

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

Looks good, only two small remarks.

web/coffee-comparison/src/browser/coffee-comparison-frontend-configuration.ts Outdated Show resolved Hide resolved
web/coffee-workflow-glsp-editor/src/browser/diagram/workflow-diagram-manager.ts Outdated Show resolved Hide resolved
eneufeld
eneufeld previously approved these changes Apr 21, 2022
View reviewed changes
Copy link
Contributor

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!
Would it make sense to add a section on how to use this to the readme?

@sgraband
Copy link
Contributor Author

I added a description for the validation and comparison to the getting started widget.

@sgraband
Copy link
Contributor Author

sgraband commented Jun 1, 2022 •
edited
Loading

I rebased the branch to master, which should resolve the issue, where nodes could not be added anymore. I also created #449 as a follow up, as it is not relevant to this change, but should be fixed next.

(Still testing in the docker container)

eneufeld
eneufeld reviewed Jul 11, 2022
View reviewed changes
Copy link
Contributor

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

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

Looks good overall.
We should split this into two commits so that we have the commit actually doing changes and one doing cleanup, eg prettier fixes

web/coffee-cpp-extension/src/browser/command-contribution.ts Show resolved Hide resolved
web/coffee-cpp-extension/src/browser/frontend-extension.ts Show resolved Hide resolved
web/coffee-cpp-extension/src/browser/generate-code-service.ts Show resolved Hide resolved
web/coffee-cpp-extension/src/node/backend-extension.ts Show resolved Hide resolved
web/coffee-cpp-extension/src/node/coffee-codegen-server.ts Show resolved Hide resolved
@sgraband sgraband requested a review from eneufeld July 12, 2022 09:00
eneufeld
eneufeld reviewed Jul 14, 2022
View reviewed changes
Copy link
Contributor

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

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

Code looks good. There are some weird formatting issues though.

...sp.server/src/org/eclipse/emfcloud/coffee/workflow/glsp/server/model/WorkflowModelState.java Outdated Show resolved Hide resolved
...sp.server/src/org/eclipse/emfcloud/coffee/workflow/glsp/server/model/WorkflowModelState.java Outdated Show resolved Hide resolved
eneufeld
eneufeld previously approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

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

Looks good. Works well.
Please squash first and third commit into one so that we have two commits in the end. Thank you!

sgraband added 2 commits July 15, 2022 09:00
@sgraband
Add model comparison
ec085b1 
Added coffee-comparison extension, that communicates via @eclipsesource/comparison-extension.
Uses git integration to retrieve head file and compare it with current state.
Update node colors to better display the comparison state.
Tree view can be triggered on the coffee file -> Compare with HEAD.
Graphical view can be triggered on the notation file -> Compare with HEAD.

Resolves eclipse-emfcloud#438.
@sgraband
Provide proper header check/template
5251db2 
Fix headers and format files.
@sgraband
Copy link
Contributor Author

Thanks for the feedback Eugen! I squashed the third commit into the first one. Could you reapprove?

@sgraband sgraband merged commit 8661ae1 into eclipse-emfcloud:master Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eneufeld eneufeld eneufeld approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add model comparison
2 participants
@sgraband @eneufeld