Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Automate Java version recommendation administrative monitor #8526
Automate Java version recommendation administrative monitor #8526
Changes from 2 commits
6dd3a0a
2c4734c
c0301a7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Jelly localization requires a
Date
object rather than the (more appropriate)LocalDate
. Here we downconvert to support Jelly localization.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.
Unused?
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.
Correct. Was there a point you were trying to make?
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.
Seems like it should be deleted if unused. A random admin monitor doesn't need to be the reference for what CSS classes exist on the UI. If there's a reason this is here while unused, it's nonobvious and should be documented in code.
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.
Well this is a model object, so just like a Hibernate model object where every field has a getter and setter, some are going to be unused. But the model object should be complete so that if someone wants to use the model in a different way then they can do so in the future. Model objects are an exception to the general wisdom around deleting unused code, and (trust me) I love deleting code — the majority of my efforts as a programmer are around code cleanup rather than greenfielding. In any case, I've never added comments to unused getters in e.g. a Hibernate model object and I do not see a compelling reason to do so here.