-
-
Notifications
You must be signed in to change notification settings - Fork 352
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
feat(model): Model change listener #1417
Conversation
70857da
to
2c7c72d
Compare
public CtCommentImpl() { | ||
} | ||
|
||
protected CtCommentImpl(CommentType type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need it for CtJavaDocImpl
rename
move addArgument to separate PR |
split ChangeFactory into ChangeFactory (empty on*) and |
a9b4b9f
to
20ca0b6
Compare
ade61b0
to
dd6daa6
Compare
Done for me. @pvojtechovsky Do you see something to improve? |
Nice code! I would not do it better. It is OK for me. Sometime I would prefer more comments ;-) I like that it is by default disabled and that it adds no overhead if it is not needed. Here are some topics for further discussion:
The topics above might be discussed/solved later - after this big PR is merged. Anyway thank You for this effort! |
09a79f5
to
9f8221c
Compare
We're almost done :-)
|
f219f44
to
d5cf834
Compare
Revapi Analysis resultsOld API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-20170627.123204-68 New API: fr.inria.gforge.spoon:spoon-core:jar:5.8.0-SNAPSHOT Detected changes: 2. Change 1
Change 2
|
Done for me |
Thanks @GerardPaligot for the intial work on this. Kudos @tdurieux for the heavyweight rebase and design improvements. |
Thanks @everybody for that great feature! |
Close #1168