-
Notifications
You must be signed in to change notification settings - Fork 6
Issue #68: Updated header recipe to prepend the expected header #72
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
Conversation
ba3b0a4 to
2b58c87
Compare
...test/resources/org/checkstyle/autofix/recipe/header/headercomments/OutputHeaderComments.java
Show resolved
Hide resolved
timurt
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.
One thought to consider
| // This file is part of the Checkstyle OpenRewrite test suite. | ||
| /////////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| /////////////////////////////////////////////////////////////////////////////////////////////// |
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 am okay with the approach, but just always prepending sounds like we might miss some opportunities
if the input file has no header -> prepend
If the input file has a header and the number of lines mismatches config header -> prepend
if the input file has a header and the number of lines is the same -> override
Something that can be considered later
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.
@Anmol202005 please create issue so we track it and refer to it. We don't have much data yet to decide what's the best way to handle this. Let's keep it as issue for later
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.
@Anmol202005 please share a link to issue
2b58c87 to
275ff68
Compare
rdiachenko
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.
lgtm
Fixes: #68
Changed recipe logic to prepend header to the existing comments.