[DISCUSS] Component labels in PR/commit messages

[Arvid Heise-4]

Dear devs,

In the last couple of weeks, I have noticed that we are slacking a bit on
the components in PR/commit messages. I'd like to gather some feedback if
we still want to include them and if so, how we can improve the process of
finding the correct label.

My personal opinion: So far, I have usually added the component because
it's in the coding guidelines. I have not really understood the benefit. It
might be coming from a time where git support in IDE was lacking and it was
necessary to maintain an overview. I also have a hard time to find the
correct component at times; I often just repeat the component that I find
in a blame. Nevertheless, I value consistency over personal taste and would
stick to the plan (and guide contributions towards it) if other devs
(especially committers) do it as well. But this has been causing some
friction in a couple of reviews for me.

Could you please give your opinion on this matter? I think it's important
to note that if long-term committers are not following it, it's really hard
for newer devs to follow that (git blame not helping in choosing the
component). Then we should remove it from the guidelines to make
contributions easier.

Thanks

Arvid

[Chesnay Schepler-3]

For commit messages the labels are useful mostly when scanning the
commit history, like searching for some commit that could've caused
something /without knowing where that change was made/, because it
enables you to quickly filter out commits by their label instead of
having to read the entire title.

I think in particular there is value in labeling documentation/build
system changes; it allows me to worry less about the phrasing because I
can assume the reader to have some context. For example,

"[FLINK-X] Remove deprecated methods" vs "[FLINK-X][docs] Remove
deprecated methods".

You could of course argue to use "[FLINK-X] Remove deprecated methods
from docs", but that's just a worse version of labeling.


On 5/19/2021 10:31 AM, Arvid Heise wrote:

> Dear devs,
>
> In the last couple of weeks, I have noticed that we are slacking a bit on
> the components in PR/commit messages. I'd like to gather some feedback if
> we still want to include them and if so, how we can improve the process of
> finding the correct label.
>
> My personal opinion: So far, I have usually added the component because
> it's in the coding guidelines. I have not really understood the benefit. It
> might be coming from a time where git support in IDE was lacking and it was
> necessary to maintain an overview. I also have a hard time to find the
> correct component at times; I often just repeat the component that I find
> in a blame. Nevertheless, I value consistency over personal taste and would
> stick to the plan (and guide contributions towards it) if other devs
> (especially committers) do it as well. But this has been causing some
> friction in a couple of reviews for me.
>
> Could you please give your opinion on this matter? I think it's important
> to note that if long-term committers are not following it, it's really hard
> for newer devs to follow that (git blame not helping in choosing the
> component). Then we should remove it from the guidelines to make
> contributions easier.
>
> Thanks
>
> Arvid
>

[Till Rohrmann]

I think a big problem with the component labels is that there is

a) no defined set of labels
b) no way to enforce the usage of them
c) no easy way to figure out which label to use

Due to these problems they are used very inconsistently in the project.

I do agree with Arvid's observation that they are less and less often used
in new commits. Given this, we could think about adjusting our guidelines
to better reflect reality and make them "optional"/"nice-to-have" for
example.

Cheers,
Till

On Wed, May 19, 2021 at 10:52 AM Chesnay Schepler <[hidden email]>
wrote:

> For commit messages the labels are useful mostly when scanning the
> commit history, like searching for some commit that could've caused
> something /without knowing where that change was made/, because it
> enables you to quickly filter out commits by their label instead of
> having to read the entire title.
>
> I think in particular there is value in labeling documentation/build
> system changes; it allows me to worry less about the phrasing because I
> can assume the reader to have some context. For example,
>
> "[FLINK-X] Remove deprecated methods" vs "[FLINK-X][docs] Remove
> deprecated methods".
>
> You could of course argue to use "[FLINK-X] Remove deprecated methods
> from docs", but that's just a worse version of labeling.
>
>
> On 5/19/2021 10:31 AM, Arvid Heise wrote:
> > Dear devs,
> >
> > In the last couple of weeks, I have noticed that we are slacking a bit on
> > the components in PR/commit messages. I'd like to gather some feedback if
> > we still want to include them and if so, how we can improve the process
> of
> > finding the correct label.
> >
> > My personal opinion: So far, I have usually added the component because
> > it's in the coding guidelines. I have not really understood the benefit.
> It
> > might be coming from a time where git support in IDE was lacking and it
> was
> > necessary to maintain an overview. I also have a hard time to find the
> > correct component at times; I often just repeat the component that I find
> > in a blame. Nevertheless, I value consistency over personal taste and
> would
> > stick to the plan (and guide contributions towards it) if other devs
> > (especially committers) do it as well. But this has been causing some
> > friction in a couple of reviews for me.
> >
> > Could you please give your opinion on this matter? I think it's important
> > to note that if long-term committers are not following it, it's really
> hard
> > for newer devs to follow that (git blame not helping in choosing the
> > component). Then we should remove it from the guidelines to make
> > contributions easier.
> >
> > Thanks
> >
> > Arvid
> >
>
>

[Robert Metzger]

+1 to Till's proposal to update the wording.

Regarding c) The guide [1] actually mentions a good heuristic for coming up
with a label that is also suitable for newcomers: The maven module name
where most of the changes are.

[1]
https://flink.apache.org/contributing/code-style-and-quality-pull-requests.html#commit-naming-conventions

On Wed, May 19, 2021 at 11:14 AM Till Rohrmann <[hidden email]> wrote:

> I think a big problem with the component labels is that there is
>
> a) no defined set of labels
> b) no way to enforce the usage of them
> c) no easy way to figure out which label to use
>
> Due to these problems they are used very inconsistently in the project.
>
> I do agree with Arvid's observation that they are less and less often used
> in new commits. Given this, we could think about adjusting our guidelines
> to better reflect reality and make them "optional"/"nice-to-have" for
> example.
>
> Cheers,
> Till
>
> On Wed, May 19, 2021 at 10:52 AM Chesnay Schepler <[hidden email]>
> wrote:
>
> > For commit messages the labels are useful mostly when scanning the
> > commit history, like searching for some commit that could've caused
> > something /without knowing where that change was made/, because it
> > enables you to quickly filter out commits by their label instead of
> > having to read the entire title.
> >
> > I think in particular there is value in labeling documentation/build
> > system changes; it allows me to worry less about the phrasing because I
> > can assume the reader to have some context. For example,
> >
> > "[FLINK-X] Remove deprecated methods" vs "[FLINK-X][docs] Remove
> > deprecated methods".
> >
> > You could of course argue to use "[FLINK-X] Remove deprecated methods
> > from docs", but that's just a worse version of labeling.
> >
> >
> > On 5/19/2021 10:31 AM, Arvid Heise wrote:
> > > Dear devs,
> > >
> > > In the last couple of weeks, I have noticed that we are slacking a bit
> on
> > > the components in PR/commit messages. I'd like to gather some feedback
> if
> > > we still want to include them and if so, how we can improve the process
> > of
> > > finding the correct label.
> > >
> > > My personal opinion: So far, I have usually added the component because
> > > it's in the coding guidelines. I have not really understood the
> benefit.
> > It
> > > might be coming from a time where git support in IDE was lacking and it
> > was
> > > necessary to maintain an overview. I also have a hard time to find the
> > > correct component at times; I often just repeat the component that I
> find
> > > in a blame. Nevertheless, I value consistency over personal taste and
> > would
> > > stick to the plan (and guide contributions towards it) if other devs
> > > (especially committers) do it as well. But this has been causing some
> > > friction in a couple of reviews for me.
> > >
> > > Could you please give your opinion on this matter? I think it's
> important
> > > to note that if long-term committers are not following it, it's really
> > hard
> > > for newer devs to follow that (git blame not helping in choosing the
> > > component). Then we should remove it from the guidelines to make
> > > contributions easier.
> > >
> > > Thanks
> > >
> > > Arvid
> > >
> >
> >
>

[roman]

Thanks for raising this issue.

I agree with the above points.
One simple argument against labels is that they consume space in the
commit messages.

+1 to make labels optional

Regards,
Roman

On Thu, May 20, 2021 at 9:31 AM Robert Metzger <[hidden email]> wrote:

>
> +1 to Till's proposal to update the wording.
>
> Regarding c) The guide [1] actually mentions a good heuristic for coming up
> with a label that is also suitable for newcomers: The maven module name
> where most of the changes are.
>
> [1]
> https://flink.apache.org/contributing/code-style-and-quality-pull-requests.html#commit-naming-conventions
>
> On Wed, May 19, 2021 at 11:14 AM Till Rohrmann <[hidden email]> wrote:
>
> > I think a big problem with the component labels is that there is
> >
> > a) no defined set of labels
> > b) no way to enforce the usage of them
> > c) no easy way to figure out which label to use
> >
> > Due to these problems they are used very inconsistently in the project.
> >
> > I do agree with Arvid's observation that they are less and less often used
> > in new commits. Given this, we could think about adjusting our guidelines
> > to better reflect reality and make them "optional"/"nice-to-have" for
> > example.
> >
> > Cheers,
> > Till
> >
> > On Wed, May 19, 2021 at 10:52 AM Chesnay Schepler <[hidden email]>
> > wrote:
> >
> > > For commit messages the labels are useful mostly when scanning the
> > > commit history, like searching for some commit that could've caused
> > > something /without knowing where that change was made/, because it
> > > enables you to quickly filter out commits by their label instead of
> > > having to read the entire title.
> > >
> > > I think in particular there is value in labeling documentation/build
> > > system changes; it allows me to worry less about the phrasing because I
> > > can assume the reader to have some context. For example,
> > >
> > > "[FLINK-X] Remove deprecated methods" vs "[FLINK-X][docs] Remove
> > > deprecated methods".
> > >
> > > You could of course argue to use "[FLINK-X] Remove deprecated methods
> > > from docs", but that's just a worse version of labeling.
> > >
> > >
> > > On 5/19/2021 10:31 AM, Arvid Heise wrote:
> > > > Dear devs,
> > > >
> > > > In the last couple of weeks, I have noticed that we are slacking a bit
> > on
> > > > the components in PR/commit messages. I'd like to gather some feedback
> > if
> > > > we still want to include them and if so, how we can improve the process
> > > of
> > > > finding the correct label.
> > > >
> > > > My personal opinion: So far, I have usually added the component because
> > > > it's in the coding guidelines. I have not really understood the
> > benefit.
> > > It
> > > > might be coming from a time where git support in IDE was lacking and it
> > > was
> > > > necessary to maintain an overview. I also have a hard time to find the
> > > > correct component at times; I often just repeat the component that I
> > find
> > > > in a blame. Nevertheless, I value consistency over personal taste and
> > > would
> > > > stick to the plan (and guide contributions towards it) if other devs
> > > > (especially committers) do it as well. But this has been causing some
> > > > friction in a couple of reviews for me.
> > > >
> > > > Could you please give your opinion on this matter? I think it's
> > important
> > > > to note that if long-term committers are not following it, it's really
> > > hard
> > > > for newer devs to follow that (git blame not helping in choosing the
> > > > component). Then we should remove it from the guidelines to make
> > > > contributions easier.
> > > >
> > > > Thanks
> > > >
> > > > Arvid
> > > >
> > >
> > >
> >