+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 >> >>
