You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/12/24 14:15:36 UTC
[GitHub] [flink] aljoscha opened a new pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
aljoscha opened a new pull request #14488:
URL: https://github.com/apache/flink/pull/14488
Please see the ML discussion for background: https://lists.apache.org/thread.html/rfb079ec4cfd35bcb93df9c2163aaa121e392282f0f3d9710c8ade811%40%3Cdev.flink.apache.org%3E. There was broad consensus in the discussion.
The basic idea is that we use the Spotless plugin to format our code with `google-java-format` and we use CI to enforce that style.
A note on checkstyle: There are still a lot of checkstyle rules that make sense and that are not covered by Spotless, such as disallowed imports and Javadocs rules.
Also, the checkstyle suppression files are still valid and checkstyle still largely fails in the few modules where we have them without them. You can remove the suppressions to get a sense of the scale of the problem. I can't fix those issues in this PR.
Note: when I'll actually merge this PR I have to rebase on latest master and reformat the code again and fix up the "ignores" file again.
## Brief change log
The changes are clearly outlined in the individual commits, please have a look at those.
Rough outline of changes:
- move inline `CHECKSTYLE:OFF` comments to supressions file because inline comments in the imports don't work with the version of `google-java-format` we're using (which we have to use because we're using an older java version for our build
- remove checkstyle checks that don't make sense after reformatting the code; it's the check for "tabs as indentation"
- add checkstyle suppressions for things that break with reformatting; this is only about file length, some files are getting to long
- fix formatting that doesn't work nicely with Spotless + Checkstyle, this is mostly about superfluous or missing `<p>` tags
- actually reformat the code
- add the Spotless plugin, I'm adding this after reformatting to ensure that the build passes at each intermediate step
- add `.git-blame-ignore-revs` for ignoring the reformatting commit
- update `.editorconfig` and update IDE setup instructions
I did the reformatting commit as `Rufus Refactor <de...@flink.apache.org>` and all the other commits as myself.
## Verifying this change
You can play around with the formatter, try and change a file and run the checks. If you "break" a file the CI build should break.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-750899577
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303",
"triggerID" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"triggerType" : "PUSH"
}, {
"hash" : "57d28f1f00ec4f019496c1f501a39d4afb2910fc",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "57d28f1f00ec4f019496c1f501a39d4afb2910fc",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 473eaeceb142acd0d983f31ee1fa55b147dfc09e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303)
* 57d28f1f00ec4f019496c1f501a39d4afb2910fc UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-750899577
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303",
"triggerID" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"triggerType" : "PUSH"
}, {
"hash" : "57d28f1f00ec4f019496c1f501a39d4afb2910fc",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11379",
"triggerID" : "57d28f1f00ec4f019496c1f501a39d4afb2910fc",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 473eaeceb142acd0d983f31ee1fa55b147dfc09e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303)
* 57d28f1f00ec4f019496c1f501a39d4afb2910fc Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11379)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol commented on a change in pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #14488:
URL: https://github.com/apache/flink/pull/14488#discussion_r549299780
##########
File path: flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/src/AbstractFileSource.java
##########
@@ -230,12 +230,14 @@ public Boundedness getBoundedness() {
/**
* The generic base builder. This builder carries a <i>SELF</i> type to make it convenient to
* extend this for subclasses, using the following pattern.
+ *
* <pre>{@code
* public class SubBuilder<T> extends AbstractFileSourceBuilder<T, SubBuilder<T>> {
* ...
* }
* }</pre>
- * That way, all return values from builder method defined here are typed to the sub-class
+ *
+ * <p>That way, all return values from builder method defined here are typed to the sub-class
Review comment:
indentation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
aljoscha commented on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-753957198
Hmm, I have the same version and cannot reproduce it. Could you try and get more verbose logs to try and see what the underlying error is?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Thesharing commented on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
Thesharing commented on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-753866464
Hello, @aljoscha, I pull the latest master branch and configure the google-java-format plugin as the [README](https://github.com/apache/flink/blob/master/docs/flinkDev/ide_setup.md) says. After that, I find that when I create a class, the IntelliJ will report an error `Unable to parse template "class"`. And when I turn off the "Reformat according to style" in the setting, the error disappears, but the new file is not formatted. Have you met the same error?
Fig. 1: the error when create a new class
<img src="https://user-images.githubusercontent.com/6576831/103520541-4729d080-4eb2-11eb-837d-2a757aa81117.png" width="500">
Fig. 2: the setting in code template
<img src="https://user-images.githubusercontent.com/6576831/103520588-56108300-4eb2-11eb-80f8-feff58088b6b.png" width=800>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-750898411
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 473eaeceb142acd0d983f31ee1fa55b147dfc09e (Thu Dec 24 14:31:17 UTC 2020)
**Warnings:**
* Documentation files were touched, but no `.zh.md` files: Update Chinese documentation or file Jira ticket.
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Thesharing commented on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
Thesharing commented on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-753894769
> For me, that works. Which version of IntellJ are you using?
The latest version, 2020.3.1.
<img src="https://user-images.githubusercontent.com/6576831/103525855-e652c600-4eba-11eb-9eea-14b7be94344e.png" width=500>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] pnowojski commented on a change in pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
pnowojski commented on a change in pull request #14488:
URL: https://github.com/apache/flink/pull/14488#discussion_r549251579
##########
File path: .git-blame-ignore-revs
##########
@@ -0,0 +1 @@
+c996ed9372fc550d0a8a97b2b986bf96d26e304c
Review comment:
```
git config blame.ignoreRevsFile .git-blame-ignore-revs
```
Can you document this somewhere? In the project's `.README`?
##########
File path: flink-streaming-java/src/test/java/org/apache/flink/streaming/api/operators/KeyedProcessOperatorTest.java
##########
@@ -6,9 +6,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- * <p>
+ *
Review comment:
Don't we require `<p>` in comments? at least in java docs? Or is it the issue, that we shouldn't have them in the license headers, but only in the java docs?
##########
File path: pom.xml
##########
@@ -1815,6 +1827,47 @@ under the License.
</configuration>
</plugin>
+ <!-- Due to the Flink build setup, "mvn spotless:apply" and "mvn spotless:check"
+ don't work. You have to use the fully qualified name, i.e.
+ "mvn com.diffplug.spotless:spotless-maven-plugin:apply" -->
Review comment:
Could this be fixed? What's causing this issue?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #14488:
URL: https://github.com/apache/flink/pull/14488#discussion_r549280620
##########
File path: .git-blame-ignore-revs
##########
@@ -0,0 +1 @@
+c996ed9372fc550d0a8a97b2b986bf96d26e304c
Review comment:
You're right, I forgot about this. I added the other setup instructions to `ide_setup.md` (c.f. https://ci.apache.org/projects/flink/flink-docs-master/flinkDev/ide_setup.html) and will also add it there.
In the long run, I think we should just add all these instructions straight to `README.md`, though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #14488:
URL: https://github.com/apache/flink/pull/14488#discussion_r549278864
##########
File path: flink-streaming-java/src/test/java/org/apache/flink/streaming/api/operators/KeyedProcessOperatorTest.java
##########
@@ -6,9 +6,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- * <p>
+ *
Review comment:
`<p>` tags are only for javadocs, and the license headers are (should be) regular comments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol merged pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
zentol merged pull request #14488:
URL: https://github.com/apache/flink/pull/14488
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
aljoscha commented on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-753890482
For me, that works. Which version of IntellJ are you using?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #14488:
URL: https://github.com/apache/flink/pull/14488#discussion_r549279115
##########
File path: pom.xml
##########
@@ -1815,6 +1827,47 @@ under the License.
</configuration>
</plugin>
+ <!-- Due to the Flink build setup, "mvn spotless:apply" and "mvn spotless:check"
+ don't work. You have to use the fully qualified name, i.e.
+ "mvn com.diffplug.spotless:spotless-maven-plugin:apply" -->
Review comment:
@zentol said that this can happen quite a lot. On a fresh Maven project `mvn spotless:apply` works, but not for us. We're stretching Maven quite a bit and that shows more and more. 🙈
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Thesharing commented on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
Thesharing commented on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-753958137
> Hmm, I have the same version and cannot reproduce it. Could you try and get more verbose logs to try and see what the underlying error is?
Okay, I'll try to work on this. Thank you so much, @aljoscha.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-750899577
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303",
"triggerID" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 473eaeceb142acd0d983f31ee1fa55b147dfc09e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-750899577
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 473eaeceb142acd0d983f31ee1fa55b147dfc09e UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14488: [FLINK-20651] Use Spotless/google-java-format for code formatting/enforcement
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14488:
URL: https://github.com/apache/flink/pull/14488#issuecomment-750899577
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303",
"triggerID" : "473eaeceb142acd0d983f31ee1fa55b147dfc09e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 473eaeceb142acd0d983f31ee1fa55b147dfc09e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11303)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org