Hi,
+1 But there are a few minor issues to watch out for: 1.Formed comments will be corrupted, example: original: * +-----------------------+ +-----------------------+ +-----------------------+ * |1 |0 |1 |1 |0 |0 |1 |1 | |1 |0 |1 |1 |0 |0 |0 |0 | |0 |1 |0 |1 |1 |0 |1 |0 | * +-----------------------+ +-----------------------+ +-----------------------+ modified: * <p>+-----------------------+ +-----------------------+ +-----------------------+ |1 |0 |1 |1 |0 * |0 |1 |1 | |1 |0 |1 |1 |0 |0 |0 |0 | |0 |1 |0 |1 |1 |0 |1 |0 | +-----------------------+ * +-----------------------+ +-----------------------+ +-----+ +-----+ +---------+ +-----+ +-----+ 2.Some license headers that use /** should be changed to /*, otherwise the content will be changed, example: original: /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information * regarding copyright ownership. The ASF licenses this file * 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 * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, * software distributed under the License is distributed on an * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * KIND, either express or implied. See the License for the * specific language governing permissions and limitations * under the License. */ modified: /** * Licensed to the Apache Software Foundation (ASF) under one or more contributor license * agreements. See the NOTICE file distributed with this work for additional information regarding * copyright ownership. The ASF licenses this file 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>http://www.apache.org/licenses/LICENSE-2.0 * * <p>Unless required by applicable law or agreed to in writing, software distributed under the * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either * express or implied. See the License for the specific language governing permissions and * limitations under the License. */ 3. Personally, lambda expressions are not very friendly to me original: localMember.getSerialToParallelPool().submit(() -> { try { long result = client.startElection(request); handler.onComplete(result); } catch (Exception e) { handler.onError(e); } finally { ClientUtils.putBackSyncHeartbeatClient(client); } }); modified: localMember .getSerialToParallelPool() .submit( () -> { try { long result = client.startElection(request); handler.onComplete(result); } catch (Exception e) { handler.onError(e); } finally { ClientUtils.putBackSyncHeartbeatClient(client); } }); Dawei On 02/16/2021 15:37，jincheng sun<[email protected]> wrote： +1 Best, Jincheng weizihan0110 <[email protected]> 于2021年2月16日周二 上午11:16写道： +1 | | Al Wei | | 邮箱：[email protected] | 签名由 网易邮箱大师 定制 On 02/16/2021 10:44, Xiangdong Huang wrote: +1. 【please pay attention】, If no -1, I will merge the PR [1] (+70K and -69K lines of codes). I have tried the commands for solving conflicts. No much work to do. [1] https://github.com/apache/iotdb/pull/2684 Best, ----------------------------------- Xiangdong Huang School of Software, Tsinghua University 黄向东 清华大学 软件学院 jincheng sun <[email protected]> 于2021年2月15日周一 下午2:32写道： Hi, I applied Spotless and Google Java format in the [1] branch, which contains two commits, one is the POM configuration of spotless, and the other is the change after the code is automatically formatted. I'm glad to see that we have automatically fixed many of our problems of mixing 2-space and 4-space in indents. I have merged one PR to spotless branch by following steps: # `asf` is repo of Apache IoTDD git fetch asf pull/2667/head:PR2667 git checkout PR2667 git cherry-pick 5ef3baaf1915095b74773b110a8f9e4e3c82429b mvn com.diffplug.spotless:spotless-maven-plugin:check mvn com.diffplug.spotless:spotless-maven-plugin:apply git add . git commit -m 'Apply spotless' git rebase asf_spotless // Resolve your conflict git rebase --continue // After squash your commits gck asf_spotless git merge PR2667 git push asf asf_spotless:spotless If you agree to apply Spotless, I will create JIRA. and OPEN the PR for apply Spotless add related documents. The document will involve some configuration instructions, such as IDEA configuration: 1. Install google-java-format plugin 2. In the plugin settings, enable the plugin and change the code style to "GOOGLE" (2-space indents) 3. Install the Save Actions plugin 4. Enable the plugin, along with "Optimize imports" and "Reformat file" 5. In the "Save Actions" settings page, setup a "File Path Inclusion" for `.*\.java`. 6. ... and other config items, such as "importOrder" etc. What are your thoughts? Best, Jincheng [1] https://github.com/apache/iotdb/tree/spotless jincheng sun <[email protected]> 于2021年2月12日周五 上午7:20写道： Hi, Thank you all for the feedback Chris，Dawei and Xiangdong! @Chris You're right. We don't want developers to pay too much attention to code styles, so I would like to make more efforts to make code styles automatic, transparent to developers (try our best), when we have finished the agreed configuration of Spotless, ` mvn spotless:check ` and `mvn spotless:apply ` can do check and correct work for us. @Dawei Thank you very much for trying and sharing the advanced functions of Spotless. About the problems you encounter, configuring the version of GoogleJavaformat should be able to solve the problem. The version 1.7 of GoogleJavaformat can work well on JDK1.8: <configuration> <java> <googleJavaFormat> <version>1.7</version> ... </googleJavaFormat> ... ... <java> </configuration> @Xiangdong You're right. Spotless will bring some changes for history code, fortunately, Spotless can automatically apply the style and helped to easily rebase the PRs, and there is an useful feature in Spotless is "ratchet" [1] With this, we can set up Spotless to only apply it's rules to files that were changed after a configured commit. This would allow a gradual application of the new coding style instead of one big change. Of course, I'd like to provide a PoC to let you see the changes after the plug-in of Spotless has applied. Thanks again for all of your feedback. I'd like to show a PoC later. Best, Jincheng [1] https://github.com/diffplug/spotless/blob/main/plugin-maven/README.md#ratchet Xiangdong Huang <[email protected]> 于2021年2月10日周三 下午10:19写道： Take away: I opt for uniforming the code style. But we have to consider how to maintain current opened PRs (we have about 40+ PRs) to solve potential conflicts. Details: I opt for uniforming the code style. In some PRs, we can find many lines are changed just because the indent is changed... Yes we are using google style, and we have a maven checkstyle plugin, indeed. If you see files under `target`, you can find the checkout-style.xml file. Or, if you add `<consoleOutput>true</consoleOutput>` to the configuration of the plugin (about line 769 in pom.xml), and run `mvn validate`, we can see many warnings on the console.. If we want to require all contributors obey the style, the " <violationSeverity>warning</violationSeverity> " configuration is needed, which will lead to the build failed if there is any violation. The current plugin does not modify the source files automatically but Spotless plugin can do that. However, if too many codes are changed, we have to consider the impact of tracing the history of codes, and we have to consider how to manually maintain current opened PRs (we have about 40+ PRs) to solve confliction. I'd like to preview the changes caused by the plugin. Can you show a patch file or a snippet? Best, ----------------------------------- Xiangdong Huang School of Software, Tsinghua University 黄向东 清华大学 软件学院 Dawei Liu <[email protected]> 于2021年2月10日周三 下午10:05写道： Hi jincheng， Thank you very much for starting this discussion I found a nice feature in the tool that automatically </removesUnusedImports>, which is a common problem in reviews because the IDE collapses imports by default. Unfortunately, it didn't work in my local (JDK 1.8), and he told me: Com/Google/googlejavaformat/Java/JavaFormatterOptions has had been compiled by a more recent version of the Java Runtime (class file version 55.0). This version of the Java Runtime only recognizes the class file versions up to 52.0 Do I need to upgrade my JDK? Dawei On 02/9/2021 16:14，Christofer Dutz<[email protected]> wrote： Perhaps it's just me, but ... If discussing formatting issues has been consuming a lot of your time, I would consider this time wasted. Especially as for newcomers this is particularly frustrating if you sort of provide the solution for a real problem and your response is "Yeah .... thanks fort hat ... but we use 4 spaces indentation and curly braces at the END of EVERY condition" ... I've been in that situation multiple times and some times just wen't away cause I just thought "WTF ... are these folks serious? I don't wanna be part of such a group". Of course would an automated plugin help. In Go I am seeing this happen auotmatically all the time. But we should ask ourselves, if paying too much attention to some formatting rules is good. But ... that's just my personal opinion. Chris -----Ursprüngliche Nachricht----- Von: jincheng sun <[email protected]> Gesendet: Dienstag, 9. Februar 2021 07:39 An: [email protected] Betreff: [DISCUSS] I would like to propose using Spotless for IoTDB coding style check Hi folks, I found that many projects use Spotless to save the time of both contributors and reviewers. I like Spotless[1] because it can be used to both check and apply a style. Then we need a formatter(e.g.: google-java-format) that works with Spotless. The pretty good thing about Spotless is that it serves as a verifier for CI but can also apply the configured style automatically. That is, for the programmer all we should to do is `mvn spotless:apply` to fix any style violations. We also have to decide on a coding style if we decide to use Spotless. and I would propose google-java-format. (It's seems IoTDB project already prefer Google's code specification, at present, iotdb uses GoogleStyle [2]) So what benefits will Spotless bring us: - No longer fussy about the coding style in reviews, which makes it easier for reviewers and contributors to understand - There is no need to fix check style errors manually because Spotless can fix them automatically Currently, Spotless has been used in many projects, such as: Apache Beam [3], Apache Flink [4], Apache Kafka [5] Apache avro [6] etc. In short words, I believe that the introduction of Spotless will save us a lot of time and focus more on the development of features. If you agree to use Spotless, I will create the JIRA. for tracking and add the detailed design and POC. What are your thoughts? --------- Finally, on the occasion of the Chinese Lunar New Year, I wish community of IoTDB better and better. I wish you a happy new year, all the best for 2021. :) ㊗️福大家，牛年大吉！ Best, Jincheng 2021.02.09 [1] https://github.com/diffplug/spotless [2] https://github.com/apache/iotdb/blob/master/java-google-style.xml [3] https://github.com/apache/beam/blob/b74fcf7b30d956fb42830d652a57b265a1546973/buildSrc/build.gradle#L23 [4] https://github.com/apache/flink/blob/4a5ef8af0bbc49d738717ae477576c172e60e62c/pom.xml#L159 [5] https://github.com/apache/kafka/blob/42a9355e606bd2bbdb7fd0dd348805e6666dc189/build.gradle#L44 [6] https://github.com/apache/avro/blob/8026c8ffe4ef67ab419dba73910636bf2c1a691c/lang/java/pom.xml#L307-L334
