You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@dolphinscheduler.apache.org by Chufeng Gao <ch...@gmail.com> on 2022/07/25 09:51:05 UTC
[Proposal] Improve code quality of DolphinScheduler
Hi community,
In this proposal, we focus on improving the code quality of the
DolphinScheduler project. There will be two main tasks to be discussed:
1. Refactoring unit tests.
2. Fixing style & formatting errors.
When those two tasks done, we expect a significant increase in code
stability and a unified coding style.
As we have already discussed in another email thread[1] and community
bi-weekly conference about replacing `Checksytle` with `Spotless` and
reached the agreement on fixing the style and formatting errors gradually,
we may combine these two tasks together.
Specifically speaking, we will submit an initial PR to add `Spotless`
plugin and enable incremental fixes on code style. Then, when we refactor
the UTs, we will fix the style and formatting errors of both the related
`test` code and `tested` code. In this way, hopefully, we will fix most of
the legacy style & formatting errors once UT refactoring done.
I ran UTs with coverage of every modules in DS by `Intellij` and generated
the reports. We will take the report into consideration when deciding the
refactoring priority. We could see the UT coverage sorted by `method
coverage` of different modules here[2].
For the criteria of UT coverage, from my perspective, I propose to use
`method coverage`. By writing tests for methods instead of a whole class,
we could make sure those tests are decoupled and will not break easily when
changing the tested code. We expect every method of DS core covered but we
may slack a bit for plugins to trade-off between code quality and
contributors' patience, as many of our contributors are tired of writing
UTs. However, we still expect contributors / maintainers to follow up those
PRs of plugins and add more UTs. Certainly there will be exceptions as some
legacy class in java are quite difficult to get covered.
There will be three subtasks for refactoring UTs:
1. Refactor UTs in each module.
2. Add some docs of instructions with examples on how to write UTs in DS.
3. Leave some typical UTs without refactored, create related issues and
label them with `good first issue` for new contributors to get onboard.
Suggestions and comments are appreciated as always and you are very welcome
to reply in this email thread : )
References and related issues:
[1]https://lists.apache.org/thread/9sqvd1wlfgkyn2y926l732zj1n3z7xkx
[2]
https://github.com/apache/dolphinscheduler/issues/10573#issuecomment-1193753867
[3]https://github.com/apache/dolphinscheduler/issues/10573
[4]https://github.com/apache/dolphinscheduler/issues/10963
*Best Regards,*
*Chufeng (Eric) Gao*
Re: [Proposal] Improve code quality of DolphinScheduler
Posted by ShunFeng Cai <ca...@gmail.com>.
Great! Thanks chufeng for working on this.
Code quality improvement is an important job, and I also think it should be
a DSIP to follow up for a long time.
--
Best Wish
— Shunfeng
Chufeng Gao <ch...@gmail.com> 于2022年7月26日周二 11:54写道：
> Hi Jiajie,
>
> Thanks for the reply.
>
> I think `we should first add some typical UTs in our test codes(from your
> third point) and then add docs to our contributor(your point two)` is a
> good suggestion. In this way, we could have more contributors easily
> participate in the early stage of this work and it helps keep things in
> track.
>
> About code quality of the future plugins, I agree with your point. It's
> always good to insist on a higher standard for the UTs especially after our
> refactoring.
>
> BTW, I suggest making this proposal a new DSIP if the community agree it
> worth one.
>
> *Best Regards,*
>
> *Chufeng (Eric) Gao*
>
>
>
> Jiajie Zhong <zh...@gmail.com> 于2022年7月26日周二 11:00写道：
>
> > Hi chufeng,
> >
> > It is a great proposal and please count with me to do it together.
> >
> > I agree that we should add `Spotless` first before we make some
> > changes to our unit tests. And your subtask also look good to me, but
> > I think the first point "Refactor UTs in each module." should be our
> > key result, and we should first add some typical UTs in our test
> > codes(from your third point) and then add docs to our contributor(your
> > point two), WDYT
> >
> > But I do not agree with your point.
> > > We expect every method of DS core covered but we may slack a bit for
> > plugins to trade-off between code quality and contributors' patience, as
> > many of our contributors are tired of writing UTs.
> >
> > I think the better way to do that it is requests contributor have to
> > add unit test and requests them cover as much as possible when they
> > contribute to DolphionScheduler, it is our responsibility to make sure
> > new code is quality, otherwise we will have other proposal to improve
> > UT code quality after we finish this, if new merged PR do not cover
> > the new adding code.
> >
> > --
> > Best Wish
> > — Jiajie
> >
>
Re: [Proposal] Improve code quality of DolphinScheduler
Posted by Jiajie Zhong <zh...@gmail.com>.
Hi chuffing,
Yesh, I agree with that, it is worth to be a new DSIP.
And BTW you are the committer now and we nomination you because we trust you. And as you can see in DSIP documents https://dolphinscheduler.apache.org/en-us/community/DSIP.html <https://dolphinscheduler.apache.org/en-us/community/DSIP.html> have a word “When the change in doubt and any committer thinks it should be DSIP, it does.”. I you think some issue worth to become DSIP, you have the right to make it as DSIP.
But you still can raise a discuss in the mail thread or ping someone in issue when you not pretty sure it worth or not.
> On Jul 26, 2022, at 11:53, Chufeng Gao <ch...@gmail.com> wrote:
>
> Hi Jiajie,
>
> Thanks for the reply.
>
> I think `we should first add some typical UTs in our test codes(from your
> third point) and then add docs to our contributor(your point two)` is a
> good suggestion. In this way, we could have more contributors easily
> participate in the early stage of this work and it helps keep things in
> track.
>
> About code quality of the future plugins, I agree with your point. It's
> always good to insist on a higher standard for the UTs especially after our
> refactoring.
>
> BTW, I suggest making this proposal a new DSIP if the community agree it
> worth one.
>
> *Best Regards,*
>
> *Chufeng (Eric) Gao*
>
>
>
> Jiajie Zhong <zh...@gmail.com> 于2022年7月26日周二 11:00写道：
>
>> Hi chufeng,
>>
>> It is a great proposal and please count with me to do it together.
>>
>> I agree that we should add `Spotless` first before we make some
>> changes to our unit tests. And your subtask also look good to me, but
>> I think the first point "Refactor UTs in each module." should be our
>> key result, and we should first add some typical UTs in our test
>> codes(from your third point) and then add docs to our contributor(your
>> point two), WDYT
>>
>> But I do not agree with your point.
>>> We expect every method of DS core covered but we may slack a bit for
>> plugins to trade-off between code quality and contributors' patience, as
>> many of our contributors are tired of writing UTs.
>>
>> I think the better way to do that it is requests contributor have to
>> add unit test and requests them cover as much as possible when they
>> contribute to DolphionScheduler, it is our responsibility to make sure
>> new code is quality, otherwise we will have other proposal to improve
>> UT code quality after we finish this, if new merged PR do not cover
>> the new adding code.
>>
>> --
>> Best Wish
>> — Jiajie
>>
Re: [Proposal] Improve code quality of DolphinScheduler
Posted by Chufeng Gao <ch...@gmail.com>.
Hi Jiajie,
Thanks for the reply.
I think `we should first add some typical UTs in our test codes(from your
third point) and then add docs to our contributor(your point two)` is a
good suggestion. In this way, we could have more contributors easily
participate in the early stage of this work and it helps keep things in
track.
About code quality of the future plugins, I agree with your point. It's
always good to insist on a higher standard for the UTs especially after our
refactoring.
BTW, I suggest making this proposal a new DSIP if the community agree it
worth one.
*Best Regards,*
*Chufeng (Eric) Gao*
Jiajie Zhong <zh...@gmail.com> 于2022年7月26日周二 11:00写道：
> Hi chufeng,
>
> It is a great proposal and please count with me to do it together.
>
> I agree that we should add `Spotless` first before we make some
> changes to our unit tests. And your subtask also look good to me, but
> I think the first point "Refactor UTs in each module." should be our
> key result, and we should first add some typical UTs in our test
> codes(from your third point) and then add docs to our contributor(your
> point two), WDYT
>
> But I do not agree with your point.
> > We expect every method of DS core covered but we may slack a bit for
> plugins to trade-off between code quality and contributors' patience, as
> many of our contributors are tired of writing UTs.
>
> I think the better way to do that it is requests contributor have to
> add unit test and requests them cover as much as possible when they
> contribute to DolphionScheduler, it is our responsibility to make sure
> new code is quality, otherwise we will have other proposal to improve
> UT code quality after we finish this, if new merged PR do not cover
> the new adding code.
>
> --
> Best Wish
> — Jiajie
>
Re: [Proposal] Improve code quality of DolphinScheduler
Posted by Jiajie Zhong <zh...@gmail.com>.
Hi chufeng,
It is a great proposal and please count with me to do it together.
I agree that we should add `Spotless` first before we make some
changes to our unit tests. And your subtask also look good to me, but
I think the first point "Refactor UTs in each module." should be our
key result, and we should first add some typical UTs in our test
codes(from your third point) and then add docs to our contributor(your
point two), WDYT
But I do not agree with your point.
> We expect every method of DS core covered but we may slack a bit for plugins to trade-off between code quality and contributors' patience, as many of our contributors are tired of writing UTs.
I think the better way to do that it is requests contributor have to
add unit test and requests them cover as much as possible when they
contribute to DolphionScheduler, it is our responsibility to make sure
new code is quality, otherwise we will have other proposal to improve
UT code quality after we finish this, if new merged PR do not cover
the new adding code.
--
Best Wish
— Jiajie