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

Enable indexing contents of diagrams #64

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

debrecenics
Copy link
Contributor

  • MD environment option is defined to enable disable this feature (as the engine needs to index a lot more elements than before, it could cause performance issues.)
  • Reference filter is updated to include the _representation feature in the indexing phase if the option is enabled.
  • ViatraQueryAdapter, MagicDrawProjectScope and BaseIndexOptions are modified accordingly.

Closes #63

@debrecenics debrecenics requested a review from ujhelyiz November 13, 2020 14:06
Copy link
Member

@ujhelyiz ujhelyiz left a comment

Choose a reason for hiding this comment

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

On the code level, this change seems reasonable.

However, @debrecenics reported in private that the _representation feature does not send correct change notifications, thus the results are not updated when the model changes - which is in my opinion a blocker issue for this change, as it goes against the ideas of the project.

@tamasborbas
Copy link
Contributor

On the code level, this change seems reasonable.

However, @debrecenics reported in private that the _representation feature does not send correct change notifications, thus the results are not updated when the model changes - which is, in my opinion, a blocker issue for this change, as it goes against the ideas of the project.

In the meantime: we really need the _representation in some cases and would accept that the incrementality will be limited. E.g. SAIC has a rule that checks if every block has an IBD diagram and the type of the diagram only stored in the AbstractDiagramRepresentationObject. We discussed with @debrecenics and @bergmanngabor that MD only sends the notifications after certain actions (e.g. saving or switching the focus to another diagram), but we could accept this. Probably the MD environment option shall show a warning about the consequences, and even when the option is enabled, we can put a warning on this feature.

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.

Content of diagrams not indexed
3 participants