Hi @Vinoth @Vino
IMO, we can use SonarQube[1] and Sonarlint[2] tools to help us detect and fix quality issues. Local env, follow below steps: ------------------------------------------------------------------------------------------ 1, docker run -d --name sonarqube -p 9000:9000 sonarqube 2, mvn sonar:sonar 3, http://localhost:9000 ------------------------------------------------------------------------------------------ [1] https://www.sonarqube.org [2] https://www.sonarlint.org Thanks Lamber-Ken At 2020-01-27 03:25:02, "Vinoth Chandar" <vin...@apache.org> wrote: >Hi Vino, > >You raise a valid point on what "MINOR" PR should be. All JIRAs start out >in "NEW" state and committers have to "Accept" the issue already (to force >early conversations like this). > >May be we should draw some bounds on it like, "cannot be more than 50 >lines", "No functionality changes" .. etc? WDYT? This seems to be the core >of the issue. > >On Thu, Jan 23, 2020 at 4:17 PM vino yang <yanghua1...@gmail.com> wrote: > >> Hi Vinoth, >> >> Thank you for your thoughts, I agree that focusing on some higher priority >> work is more valuable. >> >> This discussion is to sort out and manage the work that the community is >> already doing. There are currently some PRs working on this type of work, >> such as PR[1][2][3][4]. The community has not given guidance on these >> tasks. I think it's not very appropriate to open a "MINOR" PR directly. So, >> I want to hear from the community and how to manage them more effectively. >> The discussion does not encourage to give a higher priority to such work. >> >> We haven't stopped this kind of work, so we should provide effective >> guidance and organization so that it doesn't look disorganized. WYDT？ >> >> Best, >> Vino >> >> [1]: https://github.com/apache/incubator-hudi/pull/1237 >> [2]: https://github.com/apache/incubator-hudi/pull/1139 >> [3]: https://github.com/apache/incubator-hudi/pull/1137 >> [4]: https://github.com/apache/incubator-hudi/pull/1136 >> >> Vinoth Chandar <vin...@apache.org> 于2020年1月23日周四 下午1:20写道： >> >> > Hi, >> > >> > Thanks everyone for sharing your views! >> > >> > Some of this conversation is starting to feel like boiling the ocean. I >> > believe in refactoring with purpose and discussing class-by-class or >> > module-by-module does not make sense to me. Can we first list down what >> we >> > want to achieve? So far, I have only heard fixing IDE/IntelliJ warnings. >> > Also instead of focussing on new work, how about looking at the pending >> > JIRAs under "Testing" "Code Cleanup" components first and see if those >> are >> > worth tackling. >> > >> > We went down this path for code formatting and today we still have >> > inconsistencies. Looking back, I feel we should have clearly defined end >> > goals for the cleanups and we can then rank them based on ROI. >> > >> > Thanks >> > Vinoth >> > >> > On Wed, Jan 22, 2020 at 7:05 PM vino yang <yanghua1...@gmail.com> wrote: >> > >> > > Hi Shiyan and Bhavani: >> > > >> > > Thanks for sharing your thoughts. >> > > >> > > As I originally stated. The advantage of using modules as a unit to >> split >> > > work is that the decomposition is clear, but the disadvantage is that >> the >> > > volume of changes may be huge, which brings huge risks (considering >> that >> > > Hudi's test coverage is still not very high) and the workload of >> review. >> > > The advantage of splitting by class is that the volume of changes is >> > small >> > > and the review is more convenient, but the disadvantages are too many >> > tasks >> > > and high maintenance costs. >> > > >> > > >> > > *In addition, we need to define the boundaries of the "code cleanup" I >> > > expressed in this topic: it is limited to the smart tips shown by >> > Intellij >> > > IDEA. If the boundaries are too wide, then this discussion will lose >> > > control.* >> > > I agree with Bhavani that we don't take it as the actual goal. But we >> are >> > > not opposed to the community to help improve the quality of the code >> > > (basically, these tips given by the IDE are more reasonable). >> > > >> > > >> > > So, I still give my thoughts: We manage this work with Jira. Before we >> > > start working, we need to find a committer as a mentor. The mentor must >> > > decide whether the scale of the subtasks is reasonable and whether >> > > additional unit tests need to be added to verify the changes. And the >> > > mentor should be responsible for merged changes. >> > > >> > > What do you think? >> > > >> > > Best, >> > > Vino >> > > >> > > Bhavani Sudha <bhavanisud...@gmail.com> 于2020年1月22日周三 下午2:22写道： >> > > >> > > > Hi @vinoyang thanks for bringing this to discussion. I feel it would >> be >> > > > less disruptive to clean up code as part of individual classes being >> > > > touched for a specific goal rather than code cleanup being the actual >> > > goal. >> > > > This would narrow the touch point and ensure test coverage (both unit >> > and >> > > > integration tests) catches any accidental/unintentional changes. >> Also >> > it >> > > > would give chance to change any documentation quoting/referencing >> that >> > > > code. Wanted to share my personal opinion. >> > > > >> > > > Thanks, >> > > > Sudha >> > > > >> > > > >> > > > >> > > > On Tue, Jan 21, 2020 at 11:36 AM Shiyan Xu < >> > xu.shiyan.raym...@gmail.com> >> > > > wrote: >> > > > >> > > > > The clean-up work can actually be split by modules. >> > > > > >> > > > > Though it is generally a good practice to follow, my concern is the >> > > > > clean-up is likely to cause conflicts with some on-going changes. >> If >> > I >> > > > may >> > > > > suggest, the dedicated clean-up tasks should avoid >> > > > > - modules that are undergoing multiple feature changes/PRs >> > > > > - modules that are planned to have major refactoring due to design >> > > > changes >> > > > > (since clean-up can be done altogether during refactoring) >> > > > > >> > > > > On Tue, Jan 21, 2020 at 4:17 AM Vinoth Chandar <vin...@apache.org> >> > > > wrote: >> > > > > >> > > > > > Not sure if I fully agree with sweeping statements being made. >> But, >> > > +1 >> > > > > for >> > > > > > structuring this work via Jiras and having some committer >> “accept” >> > > the >> > > > > > issue first. Some of these tend to be subjective and we do need >> to >> > > > make >> > > > > > different tradeoffs. >> > > > > > >> > > > > > On Tue, Jan 21, 2020 at 1:28 AM vino yang <yanghua1...@gmail.com >> > >> > > > wrote: >> > > > > > >> > > > > > > Hi Pratyaksh, >> > > > > > > >> > > > > > > Thanks for your thought. >> > > > > > > >> > > > > > > Let's listen to others' comments. If there is no objection, we >> > will >> > > > > > follow >> > > > > > > this way. >> > > > > > > >> > > > > > > Best, >> > > > > > > Vino >> > > > > > > >> > > > > > > >> > > > > > > Pratyaksh Sharma <pratyaks...@gmail.com> 于2020年1月21日周二 >> 下午4:56写道： >> > > > > > > >> > > > > > > > Hi Vino, >> > > > > > > > >> > > > > > > > Big +1 for this initiative. I have done this code cleanup for >> > > test >> > > > > > > classes >> > > > > > > > in the past and strongly feel there is a need to do the same >> at >> > > > other >> > > > > > > > places as well. I would definitely like to volunteer for >> this. >> > > > > > > > >> > > > > > > > On Tue, Jan 21, 2020 at 1:52 PM vino yang < >> > yanghua1...@gmail.com >> > > > >> > > > > > wrote: >> > > > > > > > >> > > > > > > > > Hi folks, >> > > > > > > > > >> > > > > > > > > Currently, the code quality of some Hudi module is not very >> > > well. >> > > > > As >> > > > > > > many >> > > > > > > > > developers have seen, the Intellij IDEA has shown many >> > > > intellisense >> > > > > > > about >> > > > > > > > > cleanup and improvement. The community does not object to >> > doing >> > > > the >> > > > > > > > cleanup >> > > > > > > > > and improvement work and the work has been started via some >> > > > direct >> > > > > > > > "minor" >> > > > > > > > > PRs by some volunteers. The current way is unorganized and >> > hard >> > > > to >> > > > > > > > manage. >> > > > > > > > > For tracking this work, I prefer to manage this work with >> the >> > > > Jira >> > > > > > > issue. >> > > > > > > > > We can create an umbrella issue. Then, split the work into >> > > > several >> > > > > > > > > subtasks. >> > > > > > > > > >> > > > > > > > > Since those "bad smell" lays anywhere in the whole project. >> > > It's >> > > > > > > > difficult >> > > > > > > > > to give a standard to split the subtasks. For example, some >> > > files >> > > > > > have >> > > > > > > a >> > > > > > > > > lot while some modules have few. So I suggest the standard >> > > would >> > > > > > depend >> > > > > > > > on >> > > > > > > > > the volume of the changes. Before working, any subtask >> should >> > > > find >> > > > > a >> > > > > > > > > committer as a mentor who would judge and approve the scope >> > is >> > > > > > > suitable. >> > > > > > > > > >> > > > > > > > > What do you think? >> > > > > > > > > >> > > > > > > > > Any comments and suggestions would be appreciated. >> > > > > > > > > >> > > > > > > > > Best, >> > > > > > > > > Vino >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >>
