EricGao888 commented on PR #11272:
URL: 
https://github.com/apache/dolphinscheduler/pull/11272#issuecomment-1205969554

   > > For developer's convenience, I suggest setting `Spotless` executions 
configuration as follows:
   > > ```
   > > <executions>
   > > 
   > >   <execution>
   > > 
   > >       <goals>
   > > 
   > >           <goal>apply</goal>
   > > 
   > >       </goals>
   > > 
   > >       <phase>compile</phase>
   > > 
   > >   </execution>
   > > 
   > > </executions>
   > > ```
   > > 
   > > 
   > >     
   > >       
   > >     
   > > 
   > >       
   > >     
   > > 
   > >     
   > >   
   > > In this way, `Spotless` will automatically fix the style / formatting 
issues when compiling the project. However, we need to keep `Checkstyle` so 
that CI could make sure the style is correct in case developers do not compile 
and test the code before pushing it to remote repo.
   > > Another way is to use `git pre-commit hook` and remove `Checkstyle`. 
WDYT @kezhenxu94 @ruanwenjun
   > 
   > I personally used pre-commit hook in my own project and I prefer to use 
this. Also even we have the hook we should still keep the style check in CI 
because that can be skipped in developers' local machine.
   
   I think it is a good point that we should check the style in CI in case 
users skip it.
   
   To recap:
   
   1. Remove `Checkstyle` and only keep `Spotless` so that we will not need to 
maintain two style check files.
   2. Change the `execution goal` of `Spotless` from `apply` to `check` so that 
CI will check the style with `Spotless`.
   3. Encourage users to use `git pre-commit hook`.
   
   May I ask there is any best practice for setting up `git pre-commit hook`? 
Should we provide users with a script to set it up or just give them a doc? 
WDYT @kezhenxu94 @ruanwenjun 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to