Skip to content
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

[FLINK-6865] Update checkstyle documentation #4086

Closed
wants to merge 2 commits into from
Closed

Conversation

zentol
Copy link
Contributor

@zentol zentol commented Jun 7, 2017

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

@greghogan
Copy link
Contributor

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.

@zentol
Copy link
Contributor Author

zentol commented Jun 7, 2017

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.

@zentol
Copy link
Contributor Author

zentol commented Jun 7, 2017

hmm....i just retried it and it didn't work this time. Let me investigate a bit.

@zentol
Copy link
Contributor Author

zentol commented Jun 7, 2017

Something the import did in fact set was spaces around operators. It may be that not all checkstyle rules are properly imported.

@zentol
Copy link
Contributor Author

zentol commented Jun 7, 2017

Only a (small) subset of checkstyle modules can be imported, which sadly doesn't include the ImportOrder module.

@greghogan
Copy link
Contributor

Okay, second best may be to create an IntelliJ Code Style configuration for developers to import.

@zentol
Copy link
Contributor Author

zentol commented Jun 7, 2017

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.

@zentol
Copy link
Contributor Author

zentol commented Jun 7, 2017

Here's a list of the currently supported modules btw.:

EmptyLineSeparator
FileTabCharacter
Indentation
LeftCurly
LineLength
NeedBraces
NoWhitespacesBefore
WhitespaceAfter
WhitespaceAround

@zentol
Copy link
Contributor Author

zentol commented Jun 7, 2017

Apart from the ImportOrder and AvoidStartImport modules everything that we could configure via IntelliJ Code Style seems to be covered.

@zentol
Copy link
Contributor Author

zentol commented Jun 11, 2017

@greghogan With the latest plugin version (5.6.1) the import layout is adjusted as well.

@zentol
Copy link
Contributor Author

zentol commented Jun 13, 2017

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 .

- revert mentioning of strict checkstyle
- include note about not adjusted star import settings
- remove not about archetype modules as they are no longer valid
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants