About Coding Style Violations

[tianyin]

According to .github/CONTRIBUTING.md, we're using hbase-formatter.xml for controlling code style. However, much of the code is still violating the style specified in the format configuration. Relying on the IDE settings seems not very effective :(

I run CheckStyle using some of the checks specified in hbase-formatter.xml and see a number of violations. For example, we require LineLength to be 100 (specified in hbase-formatter.xml), but there're in total 48 lines longer than 100. Also, the indentation specified is 2 space (specified in hbase-formatter.xml), but Line 31 of src/main/java/redis/clients/jedis/JedisCluster.java and Line 79 of src/main/java/redis/clients/jedis/BinaryJedisCluster.java is indented with tab.

If we really want to have a consistent code style, I guess we need to use CheckStyle or something alike to enforce the code-style checking as a part of integration testing. (HBase uses CheckStyle to ensure the code style)

Thanks!

[marcosnils]

Relying on the IDE settings seems not very effective :(

we're not using IDE to format. There's a target in the makefile called make format which takes care of this.

I run CheckStyle using some of the checks specified in hbase-formatter.xml and see a number of violations.

Yes, we haven't plugged formatting in the CI yet. That's something that we should do but we haven't found the time to do it.

If we really want to have a consistent code style, I guess we need to use CheckStyle or something alike to enforce the code-style checking as a part of integration testing.

Would you like to work in this feature for Jedis?. We (the maintainers) are currently very busy with other stuff and I won't think we'll tackle this soon.

[HeartSaVioR]

Makes sense.

What we need to do it is just putting checkstyle plugin to maven pom file. That's fairly easy, but maybe need to spend another hour(s) to fix already-violated things. If we're OK with that, we can start with setting max-violation-count to current and try to reduce the number in near future.

If we want to keep following HBase codestyle, below is the checkstyle file.

https://github.com/apache/hbase/blob/master/hbase-checkstyle/src/main/resources/hbase/checkstyle.xml

[denizarsan]

I can add Checkstyle with a configuration that captures your current coding style. With it, you would have 0 violations but the configuration won't match HBase's configuration. Or I can just add the checkstyle.xml from HBase and set the max-violation-count accordingly so that you can clean the code later.

[HeartSaVioR]

Jedis codebase might not consistent for now since Jedis doesn't have code style constraints. I'd rather choose latter. Instead of keeping our style, I chose HBase formatter since we would want to adopt style guide from great project. Let's apply same decision now. @marcosnils WDYT?

[marcosnils]

I agree with @HeartSaVioR's opinion. Let's apply the current HBase format and reduce violations as a first approach.

[github-actions[bot]]

This issue is marked stale. It will be closed in 30 days if it is not updated.