-
-
Notifications
You must be signed in to change notification settings - Fork 357
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 feat: Enable comments by default #2065
Conversation
The comment was not enable for performance reasons. |
Do we have an idea about the perf difference on a large code base? |
We need to read again all the Java files. |
I'm rather OK for this new default behavior. |
THanks. We have to prepare a really good changelog with all the non-bacward compatible changes such as this one. |
if (jsapActualArgs.getBoolean("enable-comments")) { | ||
Launcher.LOGGER.warn("The option --enable-comments (-c) is deprecated as it is now the default behaviour in Spoon."); | ||
} else { | ||
Launcher.LOGGER.warn("Spoon now parse by default the comments. Consider using the option --disable-comments if you want the old behaviour."); |
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.
Typo: parse -> parses
This PR is a proposal to change a default behaviour in Spoon: until now, the default was to not take into account the comments.
I assume this behaviour was used because the comments were not properly parsed then. But I think it's not the case anymore and we got more users using Spoon for parsing comments: very recently we got a question on Stackoverflow linked to that: https://stackoverflow.com/questions/50780598/how-to-get-comments-using-spoon/ and I know we also got issues related to that.
So in the same way we decided to use
noclasspath
mode by default, I propose that we parse comments by default. WDYT?