-
Notifications
You must be signed in to change notification settings - Fork 6
Issue #50: Register the Header in recipe registry #59
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
|
changes:
|
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.
Minor comments from my side
src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java
Outdated
Show resolved
Hide resolved
src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java
Outdated
Show resolved
Hide resolved
18e2ff7 to
52d2958
Compare
|
@timurt ping |
src/test/java/org/checkstyle/autofix/CheckstyleRecipeRegistryTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/checkstyle/autofix/parser/CheckConfiguration.java
Outdated
Show resolved
Hide resolved
src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java
Outdated
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.
LGTM from my side
Thank you!
52d2958 to
3928fcf
Compare
src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java
Outdated
Show resolved
Hide resolved
10788b2 to
d564d17
Compare
|
@rdiachenko done. |
src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java
Outdated
Show resolved
Hide resolved
79dfd2c to
0ac8465
Compare
0ac8465 to
324eccb
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.
It is not ideal:
createRecipecould return optional instead of nullif (check.isPresent())could be replaced with more readable stream api likecheck.ifPresent
But we already spent too much time on this PR, and it blocks further progress. I'm ok to merge it
Fixes: #50
Implemented Header recipe in recipe registry