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

Improvements in the State-based Change Derivation #70

Merged
merged 5 commits into from
Jun 21, 2023

Conversation

h4uges
Copy link
Contributor

@h4uges h4uges commented May 8, 2023

This PR addresses #65
The AbstractStateBasedChangeResolutionStrategy has now own extension points for the used MatchEngines, PostProcessors and the used DiffEngine used by the EMFCompare instance.
DefaultStateBasedChangeResolutionStrategy extends AbstractStateBasedChangeResolutionStrategy and overwrites some of this extension points to preserve the current behavior.

h4uges added 2 commits May 8, 2023 12:54
- add AbstractStateBasedChangeResolutionStrategy with extension point for the EMFCompare to be used
- DefaultStateBasedChangeResolutionStrategy inherits now from AbstractStateBasedChangeResolutionStrategy
- add separate extension points MatchEngines, DiffEngine and Postprocessors
- further extension points for IReqEngine, IEquiEngine and IConflictDetector are possible
@h4uges h4uges requested a review from a team as a code owner May 8, 2023 12:46
@h4uges
Copy link
Contributor Author

h4uges commented May 8, 2023

AbstractStateBasedChangeResolutionStrategy is currently abstract but contains no abstract methods as it provides a default implementation. Should we change this and rename this class to BasicStateBasedChangeResolutionStrategy and remove the abstract modifier? Opinions?

@h4uges h4uges marked this pull request as draft May 8, 2023 12:51
@h4uges
Copy link
Contributor Author

h4uges commented May 8, 2023

@HansMartinA what are your thoughts on this?

@h4uges h4uges marked this pull request as ready for review May 17, 2023 06:51
@tsaglam
Copy link
Member

tsaglam commented May 17, 2023

Should we change this and rename this class to BasicStateBasedChangeResolutionStrategy and remove the abstract modifier?

Yes, I would say so. Even further: What is now the difference between the Basic one and the default one?
Can we use naming that reflects the differences?

@h4uges h4uges requested a review from HansMartinA May 22, 2023 14:49
@HansMartinA HansMartinA merged commit b83e22d into vitruv-tools:main Jun 21, 2023
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.

3 participants