You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by "damonxue (via GitHub)" <gi...@apache.org> on 2023/09/04 10:56:11 UTC
[GitHub] [shiro] damonxue opened a new pull request, #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
damonxue opened a new pull request, #1059:
URL: https://github.com/apache/shiro/pull/1059
fixes #1005
If your pull request is about ~20 lines of code you don't need to sign an [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf)
I had signed an [Individual Contributor License Agreement] in `Apache ShenYu` project.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1705059999
It's just manual labor. But CI is needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1712564168
First of all, great job, thanks!
I have fixed up the configuration for the most part to the way it was.
However, some major and minor issues remain
Some minor issues:
- Line endings. Looks like you have windows line endings in your source code. This repo uses unix line endings. Can you fix that?
Major issues:
- Tests and demos need to be included in checkstyle checking
- There are too many changes in checkstyle configuration. Most things should be done inline using `@SuppressWarnings` and not in configuration
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue closed pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue closed pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
URL: https://github.com/apache/shiro/pull/1059
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1712694956
Ok, i see. I will done the unfinished.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1714463729
You are doing a bit too much...
- Only code that fails checkstyle should be reformatted, no more no less.
- Line length should not be ignored / suppressed, but fixed. Looks like you added many suppressions for that. There should be none.
Once again I appreciate your effort, great job!
I believe this is gettin closer!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1712575740
I have started the process to fix code with the new config. Ready for you to take over and finish it. The things I did should get you started.
Thank you once again, great job!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1714895979
1 #1069
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue closed pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue closed pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
URL: https://github.com/apache/shiro/pull/1059
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1708312146
@lprimak pls review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1705794161
> there are still compiler errors :(
I think the reason is `checkstyle-plugin` in `owasp` profile. I use `mvn checkstyle:check -Powasp` to check the style, There are no compiler errors. So the question is where we should put the `checkstyle-plugin` in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1712824829
@lprimak I formated all file in line endings of `LF`, but the CI with the `windows` system run failed. Pls tell me how to do with the errors.
```
<module name="NewlineAtEndOfFile">
<property name="lineSeparator" value="lf"/>
</module>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1705824730
@lprimak pls review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1705800042
> yea... the code needs to compile has nothing to do with checkstyle. Also owasp is only secure checks, not the code style etc.
It's my fault, i did not see the `build plugins`. And did the compile error cause by the property of `root.dir`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1712968215
@lprimak @bmarwell pls review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1709258861
I ran into some backlog in my day job. Please be patient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1712576490
The only configuration that should be modified now is by adding more `JavadocPackage`s in suppressins.xml
Everything else should be made inline unless I missed something egregious
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] damonxue commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "damonxue (via GitHub)" <gi...@apache.org>.
damonxue commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1714713586
* The reason for formatting too many files is to make sure that they all end with 'lf'.
* I will change too many 'lineLength'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1705647475
there are still compiler errors :(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1705801170
No problem. The point of this issue is to fix Shiro code so it complies with the checkstyle rules and passes.
No, `root.dir` doesn't cause any issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shiro] lprimak commented on pull request #1059: [#ISSUE 1005] - Expand checkstyle to all modules (root)
Posted by "lprimak (via GitHub)" <gi...@apache.org>.
lprimak commented on PR #1059:
URL: https://github.com/apache/shiro/pull/1059#issuecomment-1705794957
yea... the code needs to compile has nothing to do with checkstyle. Also owasp is only secure checks, not the code style etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@shiro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org