-
Notifications
You must be signed in to change notification settings - Fork 914
Fixed Compile On Save in Java SE Project on application running or debugging Fixed Java Debugger did not apply the code change in Java SE Project if the project is Java source version 8 Fixed JSP Java Code analyzer in Java SE Project Improved JSP Hyperlink provider #9013
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
base: master
Are you sure you want to change the base?
Conversation
|
I'm willing to have a look at this. But these are three totally unrelated changes, that should not be in the same PR. Please split them. When creating the new PRs, please keep unrelated changes to a minimum. Whitespace without relation to the changed code should not be changed (there are exceptions to the rule, but it would be good not to go down that route), debugging only changes should be removed (there is no need to mess with log levels), commented out code should not be part of additions. Please also ensure that there is full author information available, currently there is a nickname instead of the author information. |
donphelix
left a comment
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 think we can drop these comments. With clear naming and proper spacing, the code should be self-explanatory.
|
|
||
| // private void checkPreprocessors(FileObject fileObject) { | ||
| // CssPreprocessors.getDefault().process(p, fileObject); | ||
| // } | ||
| // | ||
| // private void checkPreprocessors(FileObject fileObject, String originalName, String originalExtension) { | ||
| // CssPreprocessors.getDefault().process(p, fileObject, originalName, originalExtension); | ||
| // } |
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.
@steelswing would it make sense to remove this entirely rather than commenting it out? Might keep things cleaner.
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.
This is for the future if you need it
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.
We can create a new one.
| FileObject root = group.getRootFolder(); | ||
| objects.add(root); | ||
| } | ||
| objects.add(project.getProjectDirectory()); |
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.
Nit: blank line before the return?
|
|
||
| @Override | ||
| public void fileRenamed(FileRenameEvent fe) { | ||
| // XXX: notify BrowserReload about filename change |
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.
Nit: no need of this comment
Fixed Compile On Save in Java SE Project on application running or debugging
Fixed Java Debugger did not apply the code change in Java SE Project if the project is Java source version 8
Fixed JSP Java Code analyzer in Java SE Project
Improved JSP Hyperlink provider
^Add meaningful description above
Click to collapse/expand PR instructions
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
Please make sure (eg.
git log) that all commits have a valid name and email address for you in the Author field.If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
PR approval and merge checklist:
If this PR targets the delivery branch: don't merge. (full wiki article)