You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@echarts.apache.org by Ovilia <ov...@gmail.com> on 2022/06/06 07:19:52 UTC
[Discuss] Should we use auto-formatting tools like prettier?
Hi,
I would like to know if you think using auto-formatting tools like prettier
is a good idea.
The pros are pretty clear: It helps formatting the code in a uniformed
style without our consideration.
But for the current code that doesn't follow the style rules, we can choose
a. Fix all the code styles once in a single PR
b. Fix the code styles in a file when it is changed in a future PR
If we choose plan a, the last change of most lines would be this PR and
lose the help of "git blame".
If we choose plan b, for many times the project would have different styles
over different files and many of future PRs will have styling changings
which make the PR a little harder to review.
So I would like to know if you think the help from auto-formatting tools is
worth the price and which plan we should take for historical styling
problems.
Thanks
*Ovilia*
Re: [Discuss] Should we use auto-formatting tools like prettier?
Posted by Ovilia <ov...@gmail.com>.
Thank you all.
I think we mostly agree that we can benefit a lot from the auto-formatting
tools,
the only controversial question is if the benefit is worth the price.
Please comment
more about what you think and maybe a vote is required later to make the
final
decision.
Thanks
*Ovilia*
On Wed, Jun 8, 2022 at 3:47 PM Zhongxiang Wang <wa...@apache.org> wrote:
> Hi, I'm afraid that I have the opposite view.
> It's enough for me to follow the defined code style manually during
> the development process and if we have installed ESLint and EditorConfig
> extensions in our IDE, the tools will give us corresponding warnings or
> errors on time and even one-click fix strategies.
> I think it won't be something hard for us to fix these lint issues when we
> are making changes for a bug fix or a new feature, as we usually only need
> to modify a few specified files.
> Also, we have enabled `husky` to do the lint check before each commit.
> In a word, I personally don't second to make huge changes to fix current
> remained lint issues. They can be fixed one by one in future bug fixes and
> new features.
>
> Thanks.
>
> On Wed, Jun 8, 2022 at 2:30 PM Yi Shen <sh...@gmail.com> wrote:
>
> > I would love to use prettier in the Apache ECharts project.
> >
> > The only concern is that it will bring massive code changes and make it
> > hard to trace the git history and blame the changed code.
> >
> > On Tue, Jun 7, 2022 at 10:37 AM Ovilia <ov...@gmail.com> wrote:
> >
> > > Hi Doma,
> > >
> > > Thanks for your comments.
> > > An example of the previous git blame problems is that the last change
> of
> > > much of current code refers to
> > > the commit when we change to es6 module [1].
> > >
> > > [1]
> > >
> > >
> >
> https://github.com/apache/echarts/blame/master/src/action/roamHelper.ts#L56-L60
> > >
> > > Thanks
> > >
> > > *Ovilia*
> > >
> > >
> > > On Mon, Jun 6, 2022 at 7:15 PM Lei Shenghao <le...@126.com>
> wrote:
> > >
> > > > Hi Ovilia,
> > > >
> > > > I think it’s a good idea to use auto formatting tools which can
> prevent
> > > > whitespace changes/quote changes in PRs.
> > > >
> > > > I would suggest approach a which makes source code immediately
> benefit
> > > > from auto formatting. And it should not really spoil git-blame if it
> > > > happens in only one commit and with clear commit message, especially
> > when
> > > > the community is broadly familiar with conventional commit message.
> > > > Readers would go git-blame and find "this line was updated by the
> > commit
> > > > style: format code with prettier" and realize they should go for
> > previous
> > > > commits.
> > > >
> > > > Doma
> > > >
> > > > > 在 2022年6月6日，15:21，Ovilia <ov...@gmail.com> 写道：
> > > > >
> > > > > Hi,
> > > > >
> > > > > I would like to know if you think using auto-formatting tools like
> > > > prettier
> > > > > is a good idea.
> > > > > The pros are pretty clear: It helps formatting the code in a
> > uniformed
> > > > > style without our consideration.
> > > > >
> > > > > But for the current code that doesn't follow the style rules, we
> can
> > > > choose
> > > > > a. Fix all the code styles once in a single PR
> > > > > b. Fix the code styles in a file when it is changed in a future PR
> > > > >
> > > > > If we choose plan a, the last change of most lines would be this PR
> > and
> > > > > lose the help of "git blame".
> > > > > If we choose plan b, for many times the project would have
> different
> > > > styles
> > > > > over different files and many of future PRs will have styling
> > changings
> > > > > which make the PR a little harder to review.
> > > > >
> > > > > So I would like to know if you think the help from auto-formatting
> > > tools
> > > > is
> > > > > worth the price and which plan we should take for historical
> styling
> > > > > problems.
> > > > >
> > > > > Thanks
> > > > >
> > > > > *Ovilia*
> > > > >
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@echarts.apache.org
> > > > For additional commands, e-mail: dev-help@echarts.apache.org
> > > >
> > > >
> > >
> >
> >
> > --
> > Yi Shen
> > Apache ECharts PMC
> >
>
Re: [Discuss] Should we use auto-formatting tools like prettier?
Posted by Zhongxiang Wang <wa...@apache.org>.
Hi, I'm afraid that I have the opposite view.
It's enough for me to follow the defined code style manually during
the development process and if we have installed ESLint and EditorConfig
extensions in our IDE, the tools will give us corresponding warnings or
errors on time and even one-click fix strategies.
I think it won't be something hard for us to fix these lint issues when we
are making changes for a bug fix or a new feature, as we usually only need
to modify a few specified files.
Also, we have enabled `husky` to do the lint check before each commit.
In a word, I personally don't second to make huge changes to fix current
remained lint issues. They can be fixed one by one in future bug fixes and
new features.
Thanks.
On Wed, Jun 8, 2022 at 2:30 PM Yi Shen <sh...@gmail.com> wrote:
> I would love to use prettier in the Apache ECharts project.
>
> The only concern is that it will bring massive code changes and make it
> hard to trace the git history and blame the changed code.
>
> On Tue, Jun 7, 2022 at 10:37 AM Ovilia <ov...@gmail.com> wrote:
>
> > Hi Doma,
> >
> > Thanks for your comments.
> > An example of the previous git blame problems is that the last change of
> > much of current code refers to
> > the commit when we change to es6 module [1].
> >
> > [1]
> >
> >
> https://github.com/apache/echarts/blame/master/src/action/roamHelper.ts#L56-L60
> >
> > Thanks
> >
> > *Ovilia*
> >
> >
> > On Mon, Jun 6, 2022 at 7:15 PM Lei Shenghao <le...@126.com> wrote:
> >
> > > Hi Ovilia,
> > >
> > > I think it’s a good idea to use auto formatting tools which can prevent
> > > whitespace changes/quote changes in PRs.
> > >
> > > I would suggest approach a which makes source code immediately benefit
> > > from auto formatting. And it should not really spoil git-blame if it
> > > happens in only one commit and with clear commit message, especially
> when
> > > the community is broadly familiar with conventional commit message.
> > > Readers would go git-blame and find "this line was updated by the
> commit
> > > style: format code with prettier" and realize they should go for
> previous
> > > commits.
> > >
> > > Doma
> > >
> > > > 在 2022年6月6日，15:21，Ovilia <ov...@gmail.com> 写道：
> > > >
> > > > Hi,
> > > >
> > > > I would like to know if you think using auto-formatting tools like
> > > prettier
> > > > is a good idea.
> > > > The pros are pretty clear: It helps formatting the code in a
> uniformed
> > > > style without our consideration.
> > > >
> > > > But for the current code that doesn't follow the style rules, we can
> > > choose
> > > > a. Fix all the code styles once in a single PR
> > > > b. Fix the code styles in a file when it is changed in a future PR
> > > >
> > > > If we choose plan a, the last change of most lines would be this PR
> and
> > > > lose the help of "git blame".
> > > > If we choose plan b, for many times the project would have different
> > > styles
> > > > over different files and many of future PRs will have styling
> changings
> > > > which make the PR a little harder to review.
> > > >
> > > > So I would like to know if you think the help from auto-formatting
> > tools
> > > is
> > > > worth the price and which plan we should take for historical styling
> > > > problems.
> > > >
> > > > Thanks
> > > >
> > > > *Ovilia*
> > > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@echarts.apache.org
> > > For additional commands, e-mail: dev-help@echarts.apache.org
> > >
> > >
> >
>
>
> --
> Yi Shen
> Apache ECharts PMC
>
Re: [Discuss] Should we use auto-formatting tools like prettier?
Posted by Yi Shen <sh...@gmail.com>.
I would love to use prettier in the Apache ECharts project.
The only concern is that it will bring massive code changes and make it
hard to trace the git history and blame the changed code.
On Tue, Jun 7, 2022 at 10:37 AM Ovilia <ov...@gmail.com> wrote:
> Hi Doma,
>
> Thanks for your comments.
> An example of the previous git blame problems is that the last change of
> much of current code refers to
> the commit when we change to es6 module [1].
>
> [1]
>
> https://github.com/apache/echarts/blame/master/src/action/roamHelper.ts#L56-L60
>
> Thanks
>
> *Ovilia*
>
>
> On Mon, Jun 6, 2022 at 7:15 PM Lei Shenghao <le...@126.com> wrote:
>
> > Hi Ovilia,
> >
> > I think it’s a good idea to use auto formatting tools which can prevent
> > whitespace changes/quote changes in PRs.
> >
> > I would suggest approach a which makes source code immediately benefit
> > from auto formatting. And it should not really spoil git-blame if it
> > happens in only one commit and with clear commit message, especially when
> > the community is broadly familiar with conventional commit message.
> > Readers would go git-blame and find "this line was updated by the commit
> > style: format code with prettier" and realize they should go for previous
> > commits.
> >
> > Doma
> >
> > > 在 2022年6月6日，15:21，Ovilia <ov...@gmail.com> 写道：
> > >
> > > Hi,
> > >
> > > I would like to know if you think using auto-formatting tools like
> > prettier
> > > is a good idea.
> > > The pros are pretty clear: It helps formatting the code in a uniformed
> > > style without our consideration.
> > >
> > > But for the current code that doesn't follow the style rules, we can
> > choose
> > > a. Fix all the code styles once in a single PR
> > > b. Fix the code styles in a file when it is changed in a future PR
> > >
> > > If we choose plan a, the last change of most lines would be this PR and
> > > lose the help of "git blame".
> > > If we choose plan b, for many times the project would have different
> > styles
> > > over different files and many of future PRs will have styling changings
> > > which make the PR a little harder to review.
> > >
> > > So I would like to know if you think the help from auto-formatting
> tools
> > is
> > > worth the price and which plan we should take for historical styling
> > > problems.
> > >
> > > Thanks
> > >
> > > *Ovilia*
> > >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@echarts.apache.org
> > For additional commands, e-mail: dev-help@echarts.apache.org
> >
> >
>
--
Yi Shen
Apache ECharts PMC
Re: [Discuss] Should we use auto-formatting tools like prettier?
Posted by Ovilia <ov...@gmail.com>.
Hi Doma,
Thanks for your comments.
An example of the previous git blame problems is that the last change of
much of current code refers to
the commit when we change to es6 module [1].
[1]
https://github.com/apache/echarts/blame/master/src/action/roamHelper.ts#L56-L60
Thanks
*Ovilia*
On Mon, Jun 6, 2022 at 7:15 PM Lei Shenghao <le...@126.com> wrote:
> Hi Ovilia,
>
> I think it’s a good idea to use auto formatting tools which can prevent
> whitespace changes/quote changes in PRs.
>
> I would suggest approach a which makes source code immediately benefit
> from auto formatting. And it should not really spoil git-blame if it
> happens in only one commit and with clear commit message, especially when
> the community is broadly familiar with conventional commit message.
> Readers would go git-blame and find "this line was updated by the commit
> style: format code with prettier" and realize they should go for previous
> commits.
>
> Doma
>
> > 在 2022年6月6日，15:21，Ovilia <ov...@gmail.com> 写道：
> >
> > Hi,
> >
> > I would like to know if you think using auto-formatting tools like
> prettier
> > is a good idea.
> > The pros are pretty clear: It helps formatting the code in a uniformed
> > style without our consideration.
> >
> > But for the current code that doesn't follow the style rules, we can
> choose
> > a. Fix all the code styles once in a single PR
> > b. Fix the code styles in a file when it is changed in a future PR
> >
> > If we choose plan a, the last change of most lines would be this PR and
> > lose the help of "git blame".
> > If we choose plan b, for many times the project would have different
> styles
> > over different files and many of future PRs will have styling changings
> > which make the PR a little harder to review.
> >
> > So I would like to know if you think the help from auto-formatting tools
> is
> > worth the price and which plan we should take for historical styling
> > problems.
> >
> > Thanks
> >
> > *Ovilia*
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@echarts.apache.org
> For additional commands, e-mail: dev-help@echarts.apache.org
>
>
Re: [Discuss] Should we use auto-formatting tools like prettier?
Posted by Lei Shenghao <le...@126.com>.
Hi Ovilia,
I think it’s a good idea to use auto formatting tools which can prevent whitespace changes/quote changes in PRs.
I would suggest approach a which makes source code immediately benefit from auto formatting. And it should not really spoil git-blame if it happens in only one commit and with clear commit message, especially when the community is broadly familiar with conventional commit message. Readers would go git-blame and find "this line was updated by the commit style: format code with prettier" and realize they should go for previous commits.
Doma
> 在 2022年6月6日，15:21，Ovilia <ov...@gmail.com> 写道：
>
> Hi,
>
> I would like to know if you think using auto-formatting tools like prettier
> is a good idea.
> The pros are pretty clear: It helps formatting the code in a uniformed
> style without our consideration.
>
> But for the current code that doesn't follow the style rules, we can choose
> a. Fix all the code styles once in a single PR
> b. Fix the code styles in a file when it is changed in a future PR
>
> If we choose plan a, the last change of most lines would be this PR and
> lose the help of "git blame".
> If we choose plan b, for many times the project would have different styles
> over different files and many of future PRs will have styling changings
> which make the PR a little harder to review.
>
> So I would like to know if you think the help from auto-formatting tools is
> worth the price and which plan we should take for historical styling
> problems.
>
> Thanks
>
> *Ovilia*
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@echarts.apache.org
For additional commands, e-mail: dev-help@echarts.apache.org
Re: [Discuss] Should we use auto-formatting tools like prettier?
Posted by 唐烨男 <ty...@outlook.com>.
Hi, Ovilia!
The plan A sounds great to me. Maybe adding a special tag to the last commit before you guys formatting the whole codebase might help. Then people can checkout that tag to get blame information in that point if necessary.
The cons of formatting all the code in this famous and influential project are possibly inevitable, yet I believe the introduce of auto-formatting tools will bring a positive change which be of great benefit to developers’ experience in the future.
tyn1998
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@echarts.apache.org
For additional commands, e-mail: dev-help@echarts.apache.org
Re: [Discuss] Should we use auto-formatting tools like prettier?
Posted by Ville Brofeldt <vi...@gmail.com>.
Hi all,
I agree that the ECharts project would benefit from using a deterministic code formatter like prettier. I would vote for doing a single big PR that modifies the code. While it’s true that it will obfuscate git blame to some extent, if the commit message is clear enough, people can check what commit had changed the line in question before the reformatting pass. I have seen many other projects do something similar (I feel these types of changes are pretty much unavoidable as development tools evolve), and as such I think the benefits of making the code more readable will outweigh its disadvantages.
Ville
> On 6. Jun 2022, at 10.19, Ovilia <ov...@gmail.com> wrote:
>
> Hi,
>
> I would like to know if you think using auto-formatting tools like prettier
> is a good idea.
> The pros are pretty clear: It helps formatting the code in a uniformed
> style without our consideration.
>
> But for the current code that doesn't follow the style rules, we can choose
> a. Fix all the code styles once in a single PR
> b. Fix the code styles in a file when it is changed in a future PR
>
> If we choose plan a, the last change of most lines would be this PR and
> lose the help of "git blame".
> If we choose plan b, for many times the project would have different styles
> over different files and many of future PRs will have styling changings
> which make the PR a little harder to review.
>
> So I would like to know if you think the help from auto-formatting tools is
> worth the price and which plan we should take for historical styling
> problems.
>
> Thanks
>
> *Ovilia*
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@echarts.apache.org
For additional commands, e-mail: dev-help@echarts.apache.org