+1
BTW, We have a independent checkstyle file under iceberg module https://github.com/apache/hive/tree/master/iceberg/checkstyle . I think we need to consider unifing the checkstyle in all the sub-module. Thanks, Butao Zhang ---- Replied Message ---- | From | Zsolt Miskolczi<zsolt.miskol...@gmail.com> | | Date | 1/8/2024 16:19 | | To | <dev@hive.apache.org> | | Subject | Re: Force coding style in hive precommit | +1 In case there is an agreement about the coding style, we can prepare a tool that enforces that style at compile time. Run a tool one time to re-format all the existing code once. And turn on a compile time check. Iceberg did the same approach, they had one huge commit with almost 4k files changed and from that point, it worked well. And there are no issues about formatting. I don't think putting a warning message helps at all. Also, it should be enforced on compile time. Zsolt Kirti Ruge <kirtirug...@gmail.com> ezt írta (időpont: 2024. jan. 8., H, 7:20): +1 As it would improve maintainability and code reviews. Sometimes small indentation/styling issues would kill review cycle time and we can easily avoid it before requesting review. Enforcing more rules around it definitely boost guaranteeing quality. We can integrate it with git hooks. If we are going for this, I can work on getting it in place . Thanks, Kirti On 08-Jan-2024, at 11:36 AM, Akshat m <akshatats...@gmail.com> wrote: +1, We do have a documentation round it as well: https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConventions so it makes sense to enforce it as well. Right now we have a small section around this in documentation, We can also expand this to a new page and add more Java practices to it as well which are followed in the project while we are at this, Will be a great addition to Hive 4 documentation, I can pick it up. I suggest we add this style check as a pre-commit git hook as well, so it is enforced when the author is committing locally as well, this can save the wait time for pre-commit failure in the PR for the author to realise the styling issues, ideally this should be taken care of with the ide style configuration but in case we miss it this would error out while committing the changes. Regards, Akshat On Sat, Jan 6, 2024 at 10:17 AM László Bodor <bodorlaszlo0...@gmail.com> wrote: Hi All! What do you think about forcing coding style in Hive precommit? I remember, back in the old days, precommit printed some warnings in case some coding style (formatting, indentation, naming convention, etc.) problems were found in the patch, now it's simply not used, I guess since we're using GitHub PRs. For example: I remember I simply approved a PR a few months ago which LGTM, and later just realized it's full of 4-spaces indentation, which is wrong if we assume that code should be formatted according to the style definition here: https://github.com/apache/hive/blob/master/dev-support/eclipse-styles.xml I have just attached an example of Tez PR to open minds and start a conversation. Regards, Laszlo Bodor
