You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@seatunnel.apache.org by Jiajie Zhong <zh...@gmail.com> on 2022/02/16 03:22:10 UTC
[PROPOAL] Strict PR title when merging it
Hey guys,
I found out some of our commit title is meaningless, which you
can see in https://github.com/apache/incubator-seatunnel-website/commits/main <https://github.com/apache/incubator-seatunnel-website/commits/main>
and https://github.com/apache/incubator-seatunnel/commits/dev <https://github.com/apache/incubator-seatunnel/commits/dev>.
We have more in the website repo. I think when contributors submit PR should
make the title more meaningful. And when committer merging PR, should change
the title if not meaningful.
Some rules I think we should avoid it:
* A single word in the PR:
* https://github.com/apache/incubator-seatunnel/commit/64695611545fc459f9d0aa802dbb74ac29351a9d <https://github.com/apache/incubator-seatunnel/commit/64695611545fc459f9d0aa802dbb74ac29351a9d>
* https://github.com/apache/incubator-seatunnel-website/commit/d10241c7d038e9a3680af9694a41938bb6dfa484 <https://github.com/apache/incubator-seatunnel-website/commit/d10241c7d038e9a3680af9694a41938bb6dfa484>
* Only fix in specific issue, like `fix #123`: https://github.com/apache/incubator-seatunnel/commit/3e7d04bcbcf0224d1530a278e92bea8375d3151c <https://github.com/apache/incubator-seatunnel/commit/3e7d04bcbcf0224d1530a278e92bea8375d3151c>
We also have some best practices in
https://github.com/apache/incubator-seatunnel/commit/9268133a810f81cf168a77eb450fde74f05ad4c9 <https://github.com/apache/incubator-seatunnel/commit/9268133a810f81cf168a77eb450fde74f05ad4c9>
which includes the module an overview of this change
Cheers,
— Jiajie Zhong
Re: [PROPOAL] Strict PR title when merging it
Posted by Jiajie Zhong <zh...@gmail.com>.
Yeah
I agree with Wenun, we committers should also pay attention when we merging
PR. Changing git log makes out log short and meaningful, and also look good
when checking log
On Fri, Mar 4, 2022 at 6:27 PM Wenjun Ruan <be...@gmail.com> wrote:
>
> BTW, when committers merge PR also needs to pay attention to
> simplifying the commit message.
>
> I find there exist many duplicate commit messages in the git log.
>
> The commit message can be edited when merging PR, and remove the
> duplicate message.
>
>
> On Wed, Feb 16, 2022 at 12:01 PM Jiajie Zhong <zh...@gmail.com> wrote:
> >
> > I think is not related to the first PR title but the first line of commit message in the first commit
> >
> > Maybe using some bot or tool like git hook would help a little, but the better way to solve
> > this is we should pay more attention when we merge PR.
> >
> > Bot only makes sure we have a title. but I think only we could determine whether the title
> > is accurate or not.
> >
> > But I also like to see if we could find some bot or tool like git hook to do the first step
> > before we.
> >
> > Cheers,
> > — Jiajie Zhong
> >
> >
--
Best Wish
— Jiajie
Re: [PROPOAL] Strict PR title when merging it
Posted by Wenjun Ruan <be...@gmail.com>.
BTW, when committers merge PR also needs to pay attention to
simplifying the commit message.
I find there exist many duplicate commit messages in the git log.
The commit message can be edited when merging PR, and remove the
duplicate message.
On Wed, Feb 16, 2022 at 12:01 PM Jiajie Zhong <zh...@gmail.com> wrote:
>
> I think is not related to the first PR title but the first line of commit message in the first commit
>
> Maybe using some bot or tool like git hook would help a little, but the better way to solve
> this is we should pay more attention when we merge PR.
>
> Bot only makes sure we have a title. but I think only we could determine whether the title
> is accurate or not.
>
> But I also like to see if we could find some bot or tool like git hook to do the first step
> before we.
>
> Cheers,
> — Jiajie Zhong
>
>
Re: [PROPOAL] Strict PR title when merging it
Posted by Jiajie Zhong <zh...@gmail.com>.
I think is not related to the first PR title but the first line of commit message in the first commit
Maybe using some bot or tool like git hook would help a little, but the better way to solve
this is we should pay more attention when we merge PR.
Bot only makes sure we have a title. but I think only we could determine whether the title
is accurate or not.
But I also like to see if we could find some bot or tool like git hook to do the first step
before we.
Cheers,
— Jiajie Zhong
Re:Re:[PROPOAL] Strict PR title when merging it
Posted by leo65535 <le...@163.com>.
Hi jiajie, kirs
We can already add the following contribution check to checklist
```
Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
```
How about using git hook[1] ?
[1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks
Best wishes！
Leo65535
At 2022-02-16 11:29:19, "CalvinKirs" <ac...@163.com> wrote:
>Hi jiajie,
>Thanks for the reminder, I found a problem, it seems that the final merge will be based on the first PR-title, I will check the PR title and make changes during the merge, but sometimes I forget to change the corresponding git log header during the merge.
>Is there an automated way for us to do this?
>
>
>Best wishes！
>Calvin Kirs
>
>
>On 02/16/2022 11:22，Jiajie Zhong<zh...@gmail.com> wrote：
>Hey guys,
>
>I found out some of our commit title is meaningless, which you
>can see in https://github.com/apache/incubator-seatunnel-website/commits/main <https://github.com/apache/incubator-seatunnel-website/commits/main>
>and https://github.com/apache/incubator-seatunnel/commits/dev <https://github.com/apache/incubator-seatunnel/commits/dev>.
>
>We have more in the website repo. I think when contributors submit PR should
>make the title more meaningful. And when committer merging PR, should change
>the title if not meaningful.
>
>Some rules I think we should avoid it:
>
>* A single word in the PR:
>* https://github.com/apache/incubator-seatunnel/commit/64695611545fc459f9d0aa802dbb74ac29351a9d <https://github.com/apache/incubator-seatunnel/commit/64695611545fc459f9d0aa802dbb74ac29351a9d>
>* https://github.com/apache/incubator-seatunnel-website/commit/d10241c7d038e9a3680af9694a41938bb6dfa484 <https://github.com/apache/incubator-seatunnel-website/commit/d10241c7d038e9a3680af9694a41938bb6dfa484>
>* Only fix in specific issue, like `fix #123`: https://github.com/apache/incubator-seatunnel/commit/3e7d04bcbcf0224d1530a278e92bea8375d3151c <https://github.com/apache/incubator-seatunnel/commit/3e7d04bcbcf0224d1530a278e92bea8375d3151c>
>
>We also have some best practices in
>https://github.com/apache/incubator-seatunnel/commit/9268133a810f81cf168a77eb450fde74f05ad4c9 <https://github.com/apache/incubator-seatunnel/commit/9268133a810f81cf168a77eb450fde74f05ad4c9>
>which includes the module an overview of this change
>
>Cheers,
>— Jiajie Zhong
>
Re:[PROPOAL] Strict PR title when merging it
Posted by CalvinKirs <ac...@163.com>.
Hi jiajie,
Thanks for the reminder, I found a problem, it seems that the final merge will be based on the first PR-title, I will check the PR title and make changes during the merge, but sometimes I forget to change the corresponding git log header during the merge.
Is there an automated way for us to do this?
Best wishes！
Calvin Kirs
On 02/16/2022 11:22，Jiajie Zhong<zh...@gmail.com> wrote：
Hey guys,
I found out some of our commit title is meaningless, which you
can see in https://github.com/apache/incubator-seatunnel-website/commits/main <https://github.com/apache/incubator-seatunnel-website/commits/main>
and https://github.com/apache/incubator-seatunnel/commits/dev <https://github.com/apache/incubator-seatunnel/commits/dev>.
We have more in the website repo. I think when contributors submit PR should
make the title more meaningful. And when committer merging PR, should change
the title if not meaningful.
Some rules I think we should avoid it:
* A single word in the PR:
* https://github.com/apache/incubator-seatunnel/commit/64695611545fc459f9d0aa802dbb74ac29351a9d <https://github.com/apache/incubator-seatunnel/commit/64695611545fc459f9d0aa802dbb74ac29351a9d>
* https://github.com/apache/incubator-seatunnel-website/commit/d10241c7d038e9a3680af9694a41938bb6dfa484 <https://github.com/apache/incubator-seatunnel-website/commit/d10241c7d038e9a3680af9694a41938bb6dfa484>
* Only fix in specific issue, like `fix #123`: https://github.com/apache/incubator-seatunnel/commit/3e7d04bcbcf0224d1530a278e92bea8375d3151c <https://github.com/apache/incubator-seatunnel/commit/3e7d04bcbcf0224d1530a278e92bea8375d3151c>
We also have some best practices in
https://github.com/apache/incubator-seatunnel/commit/9268133a810f81cf168a77eb450fde74f05ad4c9 <https://github.com/apache/incubator-seatunnel/commit/9268133a810f81cf168a77eb450fde74f05ad4c9>
which includes the module an overview of this change
Cheers,
— Jiajie Zhong