You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2017/06/07 14:33:16 UTC
[GitHub] flink pull request #4086: [FLINK-6865] Update checkstyle documentation
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/4086
[FLINK-6865] Update checkstyle documentation
This PR updates the checkstyle documentation.
* The plugin setup instructions now refer to `strict-checkstyle.xml`
* Added a section for importing the checkstyle in IntelliJ for adjusting the Code Style
* Updated the section regarding modules that don't use the strict checkstyle
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink 6865
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4086.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4086
----
commit 7dcfc0cd782266fb210d01655ddd0811e7dc6e82
Author: zentol <ch...@apache.org>
Date: 2017-06-07T14:29:15Z
[FLINK-6865] Update checkstyle documentation
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
yes, it's a good idea to "flip" the checkstyle names.
I only checked a few things, but importing the checkstyle configuration, with the Checkstyle-IDEA plugin enabled, at the very least correctly configured the import order.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
I don't know a lot about the intellij code style config; can we only define subset that is imported without affecting the rest?
Btw., I'm looking into contributing to the checkstyle plugin to add support for the rules that we need.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
hmm....i just retried it and it didn't work this time. Let me investigate a bit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
Only a (small) subset of checkstyle modules can be moduled, which sadly doesn't include the `ImportOrder` module.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
@greghogan With the latest plugin version (5.6.1) the import layout is adjusted as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
Apart from the ImportOrder and AvoidStartImport modules everything that we could configure via IntelliJ Code Style seems to be covered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/4086
If the long-term plan is to have one checkstyle then we should keep the current plain name. We may be at the point where we can rename the checkstyles and update the in-progress modules to use a `relaxed-checkstyle.xml`.
I have not seen IntelliJ actually import settings from a checkstyle configuration. Not sure if this is user error or something is broken.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/4086
Okay, second best may be to create an IntelliJ Code Style configuration for developers to import.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
I've updated the PR:
- revert mentioning of strict checkstyle
- include note about not adjusted star import settings
- remove not about archetype modules as they are no longer valid
We should probably merge this after #4112 .
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
Something the import did in fact set was spaces around operators. It may be that not all checkstyle rules are properly imported.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4086
Here's a list of the currently supported modules btw.:
```
EmptyLineSeparator
FileTabCharacter
Indentation
LeftCurly
LineLength
NeedBraces
NoWhitespacesBefore
WhitespaceAfter
WhitespaceAround
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4086: [FLINK-6865] Update checkstyle documentation
Posted by zentol <gi...@git.apache.org>.
Github user zentol closed the pull request at:
https://github.com/apache/flink/pull/4086
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---