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