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

review: refactor: use LiteralHelper in DefaultJavaPrettyPrinter #1529

Merged
merged 2 commits into from
Sep 18, 2017

Conversation

pvojtechovsky
Copy link
Collaborator

Next baby step to the #1494

@pvojtechovsky
Copy link
Collaborator Author

It is ready for merge.

Note that LiteralHelper was made public, because it is used by PrinterHelper and DefaultJavaPrettyPrinter, which are in different packages.

@monperrus
Copy link
Collaborator

Interesting. This shows that class DJPP and the helpers should be in the same package. Since package printer is new and only contains 4 recent, non-API classes, I propose to move them all in package visitor and to keep them package visible.

@pvojtechovsky
Copy link
Collaborator Author

It is misunderstanding. Package printer is not new. It is old and contais PrinterHelper, which already uses all these helper classes too!

Note in future I will suggest to mark all these usages in PrinterHelper deprecated, so later helpers will be really used only from DJPP. But it is really not the case now.

@monperrus
Copy link
Collaborator

monperrus commented Sep 13, 2017 via email

@pvojtechovsky
Copy link
Collaborator Author

Did I understood well, that you suggest to move class PrinterHelper into same package like DJPP?
I like it, but I want the confirmation because it will break compatibility to old versions.

@monperrus
Copy link
Collaborator

monperrus commented Sep 13, 2017 via email

@spoon-bot
Copy link
Collaborator

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:6.0.0-20170916.224509-20

New API: fr.inria.gforge.spoon:spoon-core:jar:6.0.0-SNAPSHOT

Detected changes: 4.

Change 1

Name Element
Old class spoon.reflect.visitor.printer.ListPrinter
New none
Code java.class.removed
Description Class was removed.
Breaking binary: breaking

Change 2

Name Element
Old class spoon.reflect.visitor.printer.PrinterHelper
New none
Code java.class.removed
Description Class was removed.
Breaking binary: breaking

Change 3

Name Element
Old parameter void spoon.reflect.visitor.printer.ElementPrinterHelper::(===spoon.reflect.visitor.printer.PrinterHelper===, spoon.reflect.visitor.DefaultJavaPrettyPrinter, spoon.compiler.Environment)
New parameter void spoon.reflect.visitor.printer.ElementPrinterHelper::(===spoon.reflect.visitor.PrinterHelper===, spoon.reflect.visitor.DefaultJavaPrettyPrinter, spoon.compiler.Environment)
Code java.method.parameterTypeChanged
Description The type of the parameter changed from 'spoon.reflect.visitor.printer.PrinterHelper' to 'spoon.reflect.visitor.PrinterHelper'.
Breaking binary: breaking

Change 4

Name Element
Old parameter void spoon.reflect.visitor.printer.ElementPrinterHelper::setPrinter(===spoon.reflect.visitor.printer.PrinterHelper===)
New parameter void spoon.reflect.visitor.printer.ElementPrinterHelper::setPrinter(===spoon.reflect.visitor.PrinterHelper===)
Code java.method.parameterTypeChanged
Description The type of the parameter changed from 'spoon.reflect.visitor.printer.PrinterHelper' to 'spoon.reflect.visitor.PrinterHelper'.
Breaking binary: breaking

@pvojtechovsky
Copy link
Collaborator Author

The helpers were moved to the package of DJPP so they can stay package protected.
It is ready for merge or what else?

@monperrus
Copy link
Collaborator

OK for me, thanks. Will merge tomorrow if everybody agrees.

@monperrus monperrus merged commit 0caf179 into INRIA:master Sep 18, 2017
@pvojtechovsky pvojtechovsky deleted the refPrinterHelper2 branch September 18, 2017 19:17
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