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

Comment line fixes #280

Merged
merged 5 commits into from Mar 31, 2020
Merged

Comment line fixes #280

merged 5 commits into from Mar 31, 2020

Conversation

amolenaar
Copy link
Member

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Documentation content changes

What is the current behavior?

See #72

Issue Number: N/A

What is the new behavior?

A background service (sanitizer) ensures elements have their comments attached.

Does this PR introduce a breaking change?

  • Yes
  • No

@amolenaar amolenaar requested a review from danyeaw March 29, 2020 16:45
@amolenaar amolenaar force-pushed the comment-line-fixes branch 2 times, most recently from bfa23e3 to d74aba8 Compare March 29, 2020 20:45
Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

This looks great 👍 , I just found a couple of imports that could be cleaned up.

from gaphor.diagram.classes import ClassItem, GeneralizationItem
from gaphor.diagram.general import CommentItem, CommentLineItem
from gaphor.diagram.tests.fixtures import connect
from gaphor.services.sanitizerservice import SanitizerService
from gaphor.tests import TestCase
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove the TestCase import here.

from gaphor import UML
from gaphor.diagram.classes.generalization import GeneralizationItem
from gaphor.diagram.classes.klass import ClassItem
from gaphor.diagram.general.comment import CommentItem
from gaphor.diagram.general.commentline import CommentLineItem
from gaphor.diagram.tests.fixtures import allow, connect, disconnect
from gaphor.diagram.usecases.actor import ActorItem
from gaphor.tests import TestCase
Copy link
Member

Choose a reason for hiding this comment

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

TestCase import can be removed

@danyeaw danyeaw added the bug An issue in the application label Mar 29, 2020
@amolenaar amolenaar added this to the 1.3 milestone Mar 31, 2020
@amolenaar amolenaar merged commit 59d2ca8 into master Mar 31, 2020
@danyeaw danyeaw deleted the comment-line-fixes branch April 1, 2020 00:50
@danyeaw danyeaw modified the milestones: 1.3, 2.0 Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue in the application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants