[DISCUSS] Improve the flinkbot
Locked
12
12
Re: [DISCUSS] Improve the flinkbot
 
|
Fore code-quality/description I agree, but consensus and the final
approval should require a committer IMO. On 27.02.2019 15:08, Robert Metzger wrote: > I did not put any restrictions on who can communicate with the bot! > But since there is currently no way of overriding somebody's approval > for something, this can easily lead to such a situation. > > My thinking was that a committer still needs to manually check who > approved a pull request, and I wanted to be open for non-committers to > participate in the review process. > WIth the labels in place, this can easily send the wrong message. > > What should we do? > A) we restrict sending commands to the bot to committers? > B) only approvals from committers matter for applying labels? > C) we allow committers to override approvals > > I'm leaning towards B, as it encourages non-committers to participate. > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler <[hidden email] > <mailto:[hidden email]>> wrote: > > Just noticed that _anyone_ can approve a PR now, see > https://github.com/apache/flink/pull/7801. > > Not sure about the solution, but as it stands it is rather trivial to > nuke the review process of the entire project. > > On 13.02.2019 10:29, Robert Metzger wrote: > > Hey all, > > > > the flinkbot has been active for a week now, and I hope the > initial hiccups > > have been resolved :) > > > > I wanted to start this as a permanent thread to discuss problems and > > improvements with the bot. > > > > *So please post here if you have questions, problems or ideas how to > > improve it!* > > > |
Re: [DISCUSS] Improve the flinkbot
|
|
I will update labels only based on committer's approvals (for everything),
I think that's cleaner. We can always revisit this. On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler <[hidden email]> wrote: > Fore code-quality/description I agree, but consensus and the final > approval should require a committer IMO. > > On 27.02.2019 15:08, Robert Metzger wrote: > > I did not put any restrictions on who can communicate with the bot! > But since there is currently no way of overriding somebody's approval for > something, this can easily lead to such a situation. > > My thinking was that a committer still needs to manually check who > approved a pull request, and I wanted to be open for non-committers to > participate in the review process. > WIth the labels in place, this can easily send the wrong message. > > What should we do? > A) we restrict sending commands to the bot to committers? > B) only approvals from committers matter for applying labels? > C) we allow committers to override approvals > > I'm leaning towards B, as it encourages non-committers to participate. > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler <[hidden email]> > wrote: > >> Just noticed that _anyone_ can approve a PR now, see >> https://github.com/apache/flink/pull/7801. >> >> Not sure about the solution, but as it stands it is rather trivial to >> nuke the review process of the entire project. >> >> On 13.02.2019 10:29, Robert Metzger wrote: >> > Hey all, >> > >> > the flinkbot has been active for a week now, and I hope the initial >> hiccups >> > have been resolved :) >> > >> > I wanted to start this as a permanent thread to discuss problems and >> > improvements with the bot. >> > >> > *So please post here if you have questions, problems or ideas how to >> > improve it!* >> > >> >> > |
Re: [DISCUSS] Improve the flinkbot
|
|
It would be good to encourage participation of non-committers in the review
process, so +1 for allowing everyone to operate the bot. Github approval will show a green checkmark for committer approval (assuming accounts were linked via gitbox) - that should provide sufficient orientation? I just noticed that flinkbot seems to act as Robert when it comes to label management? I think that is confusing (besides earning Robert a lot of extra github notification mail thanks to participation on every PR :) Overall flinkbot is very useful, thanks for all the work on it! I heard positive feedback from other contributors, I think they see their contributions are better received now. Thomas On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger <[hidden email]> wrote: > I will update labels only based on committer's approvals (for everything), > I think that's cleaner. > > We can always revisit this. > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler <[hidden email]> > wrote: > > > Fore code-quality/description I agree, but consensus and the final > > approval should require a committer IMO. > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > > I did not put any restrictions on who can communicate with the bot! > > But since there is currently no way of overriding somebody's approval for > > something, this can easily lead to such a situation. > > > > My thinking was that a committer still needs to manually check who > > approved a pull request, and I wanted to be open for non-committers to > > participate in the review process. > > WIth the labels in place, this can easily send the wrong message. > > > > What should we do? > > A) we restrict sending commands to the bot to committers? > > B) only approvals from committers matter for applying labels? > > C) we allow committers to override approvals > > > > I'm leaning towards B, as it encourages non-committers to participate. > > > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler <[hidden email]> > > wrote: > > > >> Just noticed that _anyone_ can approve a PR now, see > >> https://github.com/apache/flink/pull/7801. > >> > >> Not sure about the solution, but as it stands it is rather trivial to > >> nuke the review process of the entire project. > >> > >> On 13.02.2019 10:29, Robert Metzger wrote: > >> > Hey all, > >> > > >> > the flinkbot has been active for a week now, and I hope the initial > >> hiccups > >> > have been resolved :) > >> > > >> > I wanted to start this as a permanent thread to discuss problems and > >> > improvements with the bot. > >> > > >> > *So please post here if you have questions, problems or ideas how to > >> > improve it!* > >> > > >> > >> > > > |
Re: [DISCUSS] Improve the flinkbot
|
|
GitHub has two methods for authentication with the APIs:
a) using an account's oauth token b) using the GitHub Apps API Most of the libraries for the GH API use a), so does Flinkbot. The problem with a) is that it does not allow for fine-grained access control, and Infra does not want to give Flinkbot "write" access to "apache/flink". That's why I need to rewrite parts of the bot to support b), which allows to give access only a repo's metadata, but not the code itself. On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email]> wrote: > It would be good to encourage participation of non-committers in the review > process, so +1 for allowing everyone to operate the bot. > > Github approval will show a green checkmark for committer approval > (assuming accounts were linked via gitbox) - that should provide sufficient > orientation? > > I just noticed that flinkbot seems to act as Robert when it comes to label > management? I think that is confusing (besides earning Robert a lot of > extra github notification mail thanks to participation on every PR :) > > Overall flinkbot is very useful, thanks for all the work on it! I heard > positive feedback from other contributors, I think they see their > contributions are better received now. > > Thomas > > > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger <[hidden email]> wrote: > > > I will update labels only based on committer's approvals (for > everything), > > I think that's cleaner. > > > > We can always revisit this. > > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler <[hidden email]> > > wrote: > > > > > Fore code-quality/description I agree, but consensus and the final > > > approval should require a committer IMO. > > > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > > > > I did not put any restrictions on who can communicate with the bot! > > > But since there is currently no way of overriding somebody's approval > for > > > something, this can easily lead to such a situation. > > > > > > My thinking was that a committer still needs to manually check who > > > approved a pull request, and I wanted to be open for non-committers to > > > participate in the review process. > > > WIth the labels in place, this can easily send the wrong message. > > > > > > What should we do? > > > A) we restrict sending commands to the bot to committers? > > > B) only approvals from committers matter for applying labels? > > > C) we allow committers to override approvals > > > > > > I'm leaning towards B, as it encourages non-committers to participate. > > > > > > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler <[hidden email]> > > > wrote: > > > > > >> Just noticed that _anyone_ can approve a PR now, see > > >> https://github.com/apache/flink/pull/7801. > > >> > > >> Not sure about the solution, but as it stands it is rather trivial to > > >> nuke the review process of the entire project. > > >> > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > >> > Hey all, > > >> > > > >> > the flinkbot has been active for a week now, and I hope the initial > > >> hiccups > > >> > have been resolved :) > > >> > > > >> > I wanted to start this as a permanent thread to discuss problems and > > >> > improvements with the bot. > > >> > > > >> > *So please post here if you have questions, problems or ideas how to > > >> > improve it!* > > >> > > > >> > > >> > > > > > > |
Re: [DISCUSS] Improve the flinkbot
|
|
Hi Dev,
I've been using the flinkbot and the label for a couple days, it worked really well! I have a minor suggestion, can we use different colors for different labels? We don't need to have different colors for every label, but only to distinguish whether someone had review the PR. For example, "review=description?" is the initial default label, and it may indicate that no reviewer has been try to review it. For "review=architecture?", "review=consensus?", "review=quality?", they indicate that at least someone has try to review it and approved something. It sounds like the review is in progress. For "review=approved ✅", it indicates the review is finished. So i think 3 colors is enough, it tell committers whether the review has not started yes, or in progress, or is finished. What do you think? Best, Kurt On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger <[hidden email]> wrote: > GitHub has two methods for authentication with the APIs: > a) using an account's oauth token > b) using the GitHub Apps API > > Most of the libraries for the GH API use a), so does Flinkbot. The problem > with a) is that it does not allow for fine-grained access control, and > Infra does not want to give Flinkbot "write" access to "apache/flink". > That's why I need to rewrite parts of the bot to support b), which allows > to give access only a repo's metadata, but not the code itself. > > > > > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email]> wrote: > > > It would be good to encourage participation of non-committers in the > review > > process, so +1 for allowing everyone to operate the bot. > > > > Github approval will show a green checkmark for committer approval > > (assuming accounts were linked via gitbox) - that should provide > sufficient > > orientation? > > > > I just noticed that flinkbot seems to act as Robert when it comes to > label > > management? I think that is confusing (besides earning Robert a lot of > > extra github notification mail thanks to participation on every PR :) > > > > Overall flinkbot is very useful, thanks for all the work on it! I heard > > positive feedback from other contributors, I think they see their > > contributions are better received now. > > > > Thomas > > > > > > > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger <[hidden email]> > wrote: > > > > > I will update labels only based on committer's approvals (for > > everything), > > > I think that's cleaner. > > > > > > We can always revisit this. > > > > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler <[hidden email]> > > > wrote: > > > > > > > Fore code-quality/description I agree, but consensus and the final > > > > approval should require a committer IMO. > > > > > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > > > > > > I did not put any restrictions on who can communicate with the bot! > > > > But since there is currently no way of overriding somebody's approval > > for > > > > something, this can easily lead to such a situation. > > > > > > > > My thinking was that a committer still needs to manually check who > > > > approved a pull request, and I wanted to be open for non-committers > to > > > > participate in the review process. > > > > WIth the labels in place, this can easily send the wrong message. > > > > > > > > What should we do? > > > > A) we restrict sending commands to the bot to committers? > > > > B) only approvals from committers matter for applying labels? > > > > C) we allow committers to override approvals > > > > > > > > I'm leaning towards B, as it encourages non-committers to > participate. > > > > > > > > > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler <[hidden email] > > > > > > wrote: > > > > > > > >> Just noticed that _anyone_ can approve a PR now, see > > > >> https://github.com/apache/flink/pull/7801. > > > >> > > > >> Not sure about the solution, but as it stands it is rather trivial > to > > > >> nuke the review process of the entire project. > > > >> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > > >> > Hey all, > > > >> > > > > >> > the flinkbot has been active for a week now, and I hope the > initial > > > >> hiccups > > > >> > have been resolved :) > > > >> > > > > >> > I wanted to start this as a permanent thread to discuss problems > and > > > >> > improvements with the bot. > > > >> > > > > >> > *So please post here if you have questions, problems or ideas how > to > > > >> > improve it!* > > > >> > > > > >> > > > >> > > > > > > > > > > |
Re: [DISCUSS] Improve the flinkbot
|
|
Hey Kurt, thanks a lot for this idea. My reasoning behind using just one color is the following: I wanted to use one color per category of labels. So when we are introducing labels for components, that it'll look like this: But we could of course also go with color families per category. So "review" is green colors, "component" is red colors and so on. If nobody objects (or agrees) with me, I'll change the colors soon. On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email]> wrote: Hi Dev, |
Re: [DISCUSS] Improve the flinkbot
|
|
The image didn't go through.
I would keep it as is; imo there are significantly more important things that I'd like Robert to spend time on. (literally everything in the Feature requests section) If we want to better distinguish new PRs I would suggest to either a) introduce a dedicated "New" label or b) not attach any label by default, and only attach the description label if someone has approved/disapproved it. On 06.03.2019 12:37, Robert Metzger wrote: > Hey Kurt, > thanks a lot for this idea. > > My reasoning behind using just one color is the following: I wanted to > use one color per category of labels. > So when we are introducing labels for components, that it'll look like > this: > > image.png > > But we could of course also go with color families per category. So > "review" is green colors, "component" is red colors and so on. > > If nobody objects (or agrees) with me, I'll change the colors soon. > > > On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > <mailto:[hidden email]>> wrote: > > Hi Dev, > > I've been using the flinkbot and the label for a couple days, it > worked > really well! I have a minor suggestion, can we > use different colors for different labels? We don't need to have > different > colors for every label, but only to distinguish whether > someone had review the PR. > For example, "review=description?" is the initial default label, > and it may > indicate that no reviewer has been try to review it. > > For "review=architecture?", "review=consensus?", > "review=quality?", they > indicate that at least someone has try to review it and > approved something. It sounds like the review is in progress. > > For "review=approved ✅", it indicates the review is finished. > > So i think 3 colors is enough, it tell committers whether the > review has > not started yes, or in progress, or is finished. > > What do you think? > > Best, > Kurt > > > On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger <[hidden email] > <mailto:[hidden email]>> wrote: > > > GitHub has two methods for authentication with the APIs: > > a) using an account's oauth token > > b) using the GitHub Apps API > > > > Most of the libraries for the GH API use a), so does Flinkbot. > The problem > > with a) is that it does not allow for fine-grained access > control, and > > Infra does not want to give Flinkbot "write" access to > "apache/flink". > > That's why I need to rewrite parts of the bot to support b), > which allows > > to give access only a repo's metadata, but not the code itself. > > > > > > > > > > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email] > <mailto:[hidden email]>> wrote: > > > > > It would be good to encourage participation of non-committers > in the > > review > > > process, so +1 for allowing everyone to operate the bot. > > > > > > Github approval will show a green checkmark for committer approval > > > (assuming accounts were linked via gitbox) - that should provide > > sufficient > > > orientation? > > > > > > I just noticed that flinkbot seems to act as Robert when it > comes to > > label > > > management? I think that is confusing (besides earning Robert > a lot of > > > extra github notification mail thanks to participation on > every PR :) > > > > > > Overall flinkbot is very useful, thanks for all the work on > it! I heard > > > positive feedback from other contributors, I think they see their > > > contributions are better received now. > > > > > > Thomas > > > > > > > > > > > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > <[hidden email] <mailto:[hidden email]>> > > wrote: > > > > > > > I will update labels only based on committer's approvals (for > > > everything), > > > > I think that's cleaner. > > > > > > > > We can always revisit this. > > > > > > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > <[hidden email] <mailto:[hidden email]>> > > > > wrote: > > > > > > > > > Fore code-quality/description I agree, but consensus and > the final > > > > > approval should require a committer IMO. > > > > > > > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > > > > > > > > I did not put any restrictions on who can communicate with > the bot! > > > > > But since there is currently no way of overriding > somebody's approval > > > for > > > > > something, this can easily lead to such a situation. > > > > > > > > > > My thinking was that a committer still needs to manually > check who > > > > > approved a pull request, and I wanted to be open for > non-committers > > to > > > > > participate in the review process. > > > > > WIth the labels in place, this can easily send the wrong > message. > > > > > > > > > > What should we do? > > > > > A) we restrict sending commands to the bot to committers? > > > > > B) only approvals from committers matter for applying labels? > > > > > C) we allow committers to override approvals > > > > > > > > > > I'm leaning towards B, as it encourages non-committers to > > participate. > > > > > > > > > > > > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler > <[hidden email] <mailto:[hidden email]> > > > > > > > > wrote: > > > > > > > > > >> Just noticed that _anyone_ can approve a PR now, see > > > > >> https://github.com/apache/flink/pull/7801. > > > > >> > > > > >> Not sure about the solution, but as it stands it is > rather trivial > > to > > > > >> nuke the review process of the entire project. > > > > >> > > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > > > >> > Hey all, > > > > >> > > > > > >> > the flinkbot has been active for a week now, and I hope the > > initial > > > > >> hiccups > > > > >> > have been resolved :) > > > > >> > > > > > >> > I wanted to start this as a permanent thread to discuss > problems > > and > > > > >> > improvements with the bot. > > > > >> > > > > > >> > *So please post here if you have questions, problems or > ideas how > > to > > > > >> > improve it!* > > > > >> > > > > > >> > > > > >> > > > > > > > > > > > > > > > |
Re: [DISCUSS] Improve the flinkbot
|
|
This is the picture:
https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png Speaking about feature requests, priorities and time-spend: My plan was to now work on introducing a new label category for the components. This should get us a lot better overview over the per-component status/health of pull requests. On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler <[hidden email]> wrote: > The image didn't go through. > > I would keep it as is; imo there are significantly more important things > that I'd like Robert to spend time on. (literally everything in the > Feature requests section) > > If we want to better distinguish new PRs I would suggest to either a) > introduce a dedicated "New" label or b) not attach any label by default, > and only attach the description label if someone has > approved/disapproved it. > > On 06.03.2019 12:37, Robert Metzger wrote: > > Hey Kurt, > > thanks a lot for this idea. > > > > My reasoning behind using just one color is the following: I wanted to > > use one color per category of labels. > > So when we are introducing labels for components, that it'll look like > > this: > > > > image.png > > > > But we could of course also go with color families per category. So > > "review" is green colors, "component" is red colors and so on. > > > > If nobody objects (or agrees) with me, I'll change the colors soon. > > > > > > On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > > <mailto:[hidden email]>> wrote: > > > > Hi Dev, > > > > I've been using the flinkbot and the label for a couple days, it > > worked > > really well! I have a minor suggestion, can we > > use different colors for different labels? We don't need to have > > different > > colors for every label, but only to distinguish whether > > someone had review the PR. > > For example, "review=description?" is the initial default label, > > and it may > > indicate that no reviewer has been try to review it. > > > > For "review=architecture?", "review=consensus?", > > "review=quality?", they > > indicate that at least someone has try to review it and > > approved something. It sounds like the review is in progress. > > > > For "review=approved ✅", it indicates the review is finished. > > > > So i think 3 colors is enough, it tell committers whether the > > review has > > not started yes, or in progress, or is finished. > > > > What do you think? > > > > Best, > > Kurt > > > > > > On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger <[hidden email] > > <mailto:[hidden email]>> wrote: > > > > > GitHub has two methods for authentication with the APIs: > > > a) using an account's oauth token > > > b) using the GitHub Apps API > > > > > > Most of the libraries for the GH API use a), so does Flinkbot. > > The problem > > > with a) is that it does not allow for fine-grained access > > control, and > > > Infra does not want to give Flinkbot "write" access to > > "apache/flink". > > > That's why I need to rewrite parts of the bot to support b), > > which allows > > > to give access only a repo's metadata, but not the code itself. > > > > > > > > > > > > > > > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email] > > <mailto:[hidden email]>> wrote: > > > > > > > It would be good to encourage participation of non-committers > > in the > > > review > > > > process, so +1 for allowing everyone to operate the bot. > > > > > > > > Github approval will show a green checkmark for committer > approval > > > > (assuming accounts were linked via gitbox) - that should provide > > > sufficient > > > > orientation? > > > > > > > > I just noticed that flinkbot seems to act as Robert when it > > comes to > > > label > > > > management? I think that is confusing (besides earning Robert > > a lot of > > > > extra github notification mail thanks to participation on > > every PR :) > > > > > > > > Overall flinkbot is very useful, thanks for all the work on > > it! I heard > > > > positive feedback from other contributors, I think they see their > > > > contributions are better received now. > > > > > > > > Thomas > > > > > > > > > > > > > > > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > > <[hidden email] <mailto:[hidden email]>> > > > wrote: > > > > > > > > > I will update labels only based on committer's approvals (for > > > > everything), > > > > > I think that's cleaner. > > > > > > > > > > We can always revisit this. > > > > > > > > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > > <[hidden email] <mailto:[hidden email]>> > > > > > wrote: > > > > > > > > > > > Fore code-quality/description I agree, but consensus and > > the final > > > > > > approval should require a committer IMO. > > > > > > > > > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > > > > > > > > > > I did not put any restrictions on who can communicate with > > the bot! > > > > > > But since there is currently no way of overriding > > somebody's approval > > > > for > > > > > > something, this can easily lead to such a situation. > > > > > > > > > > > > My thinking was that a committer still needs to manually > > check who > > > > > > approved a pull request, and I wanted to be open for > > non-committers > > > to > > > > > > participate in the review process. > > > > > > WIth the labels in place, this can easily send the wrong > > message. > > > > > > > > > > > > What should we do? > > > > > > A) we restrict sending commands to the bot to committers? > > > > > > B) only approvals from committers matter for applying labels? > > > > > > C) we allow committers to override approvals > > > > > > > > > > > > I'm leaning towards B, as it encourages non-committers to > > > participate. > > > > > > > > > > > > > > > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler > > <[hidden email] <mailto:[hidden email]> > > > > > > > > > > wrote: > > > > > > > > > > > >> Just noticed that _anyone_ can approve a PR now, see > > > > > >> https://github.com/apache/flink/pull/7801. > > > > > >> > > > > > >> Not sure about the solution, but as it stands it is > > rather trivial > > > to > > > > > >> nuke the review process of the entire project. > > > > > >> > > > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > > > > >> > Hey all, > > > > > >> > > > > > > >> > the flinkbot has been active for a week now, and I hope > the > > > initial > > > > > >> hiccups > > > > > >> > have been resolved :) > > > > > >> > > > > > > >> > I wanted to start this as a permanent thread to discuss > > problems > > > and > > > > > >> > improvements with the bot. > > > > > >> > > > > > > >> > *So please post here if you have questions, problems or > > ideas how > > > to > > > > > >> > improve it!* > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > > > > > > |
Re: [DISCUSS] Improve the flinkbot
|
|
Component labels seem a bit redundant. Every JIRA with an open PR
already has a "pull-request-available" tag. So this information already exists. I assume you'll base the labels on the component tags at the time the PR is opened, but this also implies that they may be set incorrectly (or not at all) by the contributor. In this case we now have to update the component both in JIRA and on GitHub, and I'm most certainly not looking forward to that. On 06.03.2019 13:51, Robert Metzger wrote: > This is the picture: > https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png > > Speaking about feature requests, priorities and time-spend: My plan was to > now work on introducing a new label category for the components. > This should get us a lot better overview over the per-component > status/health of pull requests. > > > On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler <[hidden email]> wrote: > >> The image didn't go through. >> >> I would keep it as is; imo there are significantly more important things >> that I'd like Robert to spend time on. (literally everything in the >> Feature requests section) >> >> If we want to better distinguish new PRs I would suggest to either a) >> introduce a dedicated "New" label or b) not attach any label by default, >> and only attach the description label if someone has >> approved/disapproved it. >> >> On 06.03.2019 12:37, Robert Metzger wrote: >>> Hey Kurt, >>> thanks a lot for this idea. >>> >>> My reasoning behind using just one color is the following: I wanted to >>> use one color per category of labels. >>> So when we are introducing labels for components, that it'll look like >>> this: >>> >>> image.png >>> >>> But we could of course also go with color families per category. So >>> "review" is green colors, "component" is red colors and so on. >>> >>> If nobody objects (or agrees) with me, I'll change the colors soon. >>> >>> >>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] >>> <mailto:[hidden email]>> wrote: >>> >>> Hi Dev, >>> >>> I've been using the flinkbot and the label for a couple days, it >>> worked >>> really well! I have a minor suggestion, can we >>> use different colors for different labels? We don't need to have >>> different >>> colors for every label, but only to distinguish whether >>> someone had review the PR. >>> For example, "review=description?" is the initial default label, >>> and it may >>> indicate that no reviewer has been try to review it. >>> >>> For "review=architecture?", "review=consensus?", >>> "review=quality?", they >>> indicate that at least someone has try to review it and >>> approved something. It sounds like the review is in progress. >>> >>> For "review=approved ✅", it indicates the review is finished. >>> >>> So i think 3 colors is enough, it tell committers whether the >>> review has >>> not started yes, or in progress, or is finished. >>> >>> What do you think? >>> >>> Best, >>> Kurt >>> >>> >>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger <[hidden email] >>> <mailto:[hidden email]>> wrote: >>> >>> > GitHub has two methods for authentication with the APIs: >>> > a) using an account's oauth token >>> > b) using the GitHub Apps API >>> > >>> > Most of the libraries for the GH API use a), so does Flinkbot. >>> The problem >>> > with a) is that it does not allow for fine-grained access >>> control, and >>> > Infra does not want to give Flinkbot "write" access to >>> "apache/flink". >>> > That's why I need to rewrite parts of the bot to support b), >>> which allows >>> > to give access only a repo's metadata, but not the code itself. >>> > >>> > >>> > >>> > >>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email] >>> <mailto:[hidden email]>> wrote: >>> > >>> > > It would be good to encourage participation of non-committers >>> in the >>> > review >>> > > process, so +1 for allowing everyone to operate the bot. >>> > > >>> > > Github approval will show a green checkmark for committer >> approval >>> > > (assuming accounts were linked via gitbox) - that should provide >>> > sufficient >>> > > orientation? >>> > > >>> > > I just noticed that flinkbot seems to act as Robert when it >>> comes to >>> > label >>> > > management? I think that is confusing (besides earning Robert >>> a lot of >>> > > extra github notification mail thanks to participation on >>> every PR :) >>> > > >>> > > Overall flinkbot is very useful, thanks for all the work on >>> it! I heard >>> > > positive feedback from other contributors, I think they see their >>> > > contributions are better received now. >>> > > >>> > > Thomas >>> > > >>> > > >>> > > >>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger >>> <[hidden email] <mailto:[hidden email]>> >>> > wrote: >>> > > >>> > > > I will update labels only based on committer's approvals (for >>> > > everything), >>> > > > I think that's cleaner. >>> > > > >>> > > > We can always revisit this. >>> > > > >>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler >>> <[hidden email] <mailto:[hidden email]>> >>> > > > wrote: >>> > > > >>> > > > > Fore code-quality/description I agree, but consensus and >>> the final >>> > > > > approval should require a committer IMO. >>> > > > > >>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: >>> > > > > >>> > > > > I did not put any restrictions on who can communicate with >>> the bot! >>> > > > > But since there is currently no way of overriding >>> somebody's approval >>> > > for >>> > > > > something, this can easily lead to such a situation. >>> > > > > >>> > > > > My thinking was that a committer still needs to manually >>> check who >>> > > > > approved a pull request, and I wanted to be open for >>> non-committers >>> > to >>> > > > > participate in the review process. >>> > > > > WIth the labels in place, this can easily send the wrong >>> message. >>> > > > > >>> > > > > What should we do? >>> > > > > A) we restrict sending commands to the bot to committers? >>> > > > > B) only approvals from committers matter for applying labels? >>> > > > > C) we allow committers to override approvals >>> > > > > >>> > > > > I'm leaning towards B, as it encourages non-committers to >>> > participate. >>> > > > > >>> > > > > >>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler >>> <[hidden email] <mailto:[hidden email]> >>> > > >>> > > > > wrote: >>> > > > > >>> > > > >> Just noticed that _anyone_ can approve a PR now, see >>> > > > >> https://github.com/apache/flink/pull/7801. >>> > > > >> >>> > > > >> Not sure about the solution, but as it stands it is >>> rather trivial >>> > to >>> > > > >> nuke the review process of the entire project. >>> > > > >> >>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: >>> > > > >> > Hey all, >>> > > > >> > >>> > > > >> > the flinkbot has been active for a week now, and I hope >> the >>> > initial >>> > > > >> hiccups >>> > > > >> > have been resolved :) >>> > > > >> > >>> > > > >> > I wanted to start this as a permanent thread to discuss >>> problems >>> > and >>> > > > >> > improvements with the bot. >>> > > > >> > >>> > > > >> > *So please post here if you have questions, problems or >>> ideas how >>> > to >>> > > > >> > improve it!* >>> > > > >> > >>> > > > >> >>> > > > >> >>> > > > > >>> > > > >>> > > >>> > >>> >> |
Re: [DISCUSS] Improve the flinkbot
|
|
I will automatically assign the Jira component as a label to the PR, yes.
You won't have to manually update the label on the PR, this will be done automatically. So JIRA will stay the ground truth for setting the component correctly. On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler <[hidden email]> wrote: > Component labels seem a bit redundant. Every JIRA with an open PR > already has a "pull-request-available" tag. > So this information already exists. > > I assume you'll base the labels on the component tags at the time the PR > is opened, but this also implies that they may be set incorrectly (or > not at all) by the contributor. In this case we now have to update the > component both in JIRA and on GitHub, and I'm most certainly not looking > forward to that. > > On 06.03.2019 13:51, Robert Metzger wrote: > > This is the picture: > > > https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png > > > > Speaking about feature requests, priorities and time-spend: My plan was > to > > now work on introducing a new label category for the components. > > This should get us a lot better overview over the per-component > > status/health of pull requests. > > > > > > On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler <[hidden email]> > wrote: > > > >> The image didn't go through. > >> > >> I would keep it as is; imo there are significantly more important things > >> that I'd like Robert to spend time on. (literally everything in the > >> Feature requests section) > >> > >> If we want to better distinguish new PRs I would suggest to either a) > >> introduce a dedicated "New" label or b) not attach any label by default, > >> and only attach the description label if someone has > >> approved/disapproved it. > >> > >> On 06.03.2019 12:37, Robert Metzger wrote: > >>> Hey Kurt, > >>> thanks a lot for this idea. > >>> > >>> My reasoning behind using just one color is the following: I wanted to > >>> use one color per category of labels. > >>> So when we are introducing labels for components, that it'll look like > >>> this: > >>> > >>> image.png > >>> > >>> But we could of course also go with color families per category. So > >>> "review" is green colors, "component" is red colors and so on. > >>> > >>> If nobody objects (or agrees) with me, I'll change the colors soon. > >>> > >>> > >>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > >>> <mailto:[hidden email]>> wrote: > >>> > >>> Hi Dev, > >>> > >>> I've been using the flinkbot and the label for a couple days, it > >>> worked > >>> really well! I have a minor suggestion, can we > >>> use different colors for different labels? We don't need to have > >>> different > >>> colors for every label, but only to distinguish whether > >>> someone had review the PR. > >>> For example, "review=description?" is the initial default label, > >>> and it may > >>> indicate that no reviewer has been try to review it. > >>> > >>> For "review=architecture?", "review=consensus?", > >>> "review=quality?", they > >>> indicate that at least someone has try to review it and > >>> approved something. It sounds like the review is in progress. > >>> > >>> For "review=approved ✅", it indicates the review is finished. > >>> > >>> So i think 3 colors is enough, it tell committers whether the > >>> review has > >>> not started yes, or in progress, or is finished. > >>> > >>> What do you think? > >>> > >>> Best, > >>> Kurt > >>> > >>> > >>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < > [hidden email] > >>> <mailto:[hidden email]>> wrote: > >>> > >>> > GitHub has two methods for authentication with the APIs: > >>> > a) using an account's oauth token > >>> > b) using the GitHub Apps API > >>> > > >>> > Most of the libraries for the GH API use a), so does Flinkbot. > >>> The problem > >>> > with a) is that it does not allow for fine-grained access > >>> control, and > >>> > Infra does not want to give Flinkbot "write" access to > >>> "apache/flink". > >>> > That's why I need to rewrite parts of the bot to support b), > >>> which allows > >>> > to give access only a repo's metadata, but not the code itself. > >>> > > >>> > > >>> > > >>> > > >>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email] > >>> <mailto:[hidden email]>> wrote: > >>> > > >>> > > It would be good to encourage participation of non-committers > >>> in the > >>> > review > >>> > > process, so +1 for allowing everyone to operate the bot. > >>> > > > >>> > > Github approval will show a green checkmark for committer > >> approval > >>> > > (assuming accounts were linked via gitbox) - that should > provide > >>> > sufficient > >>> > > orientation? > >>> > > > >>> > > I just noticed that flinkbot seems to act as Robert when it > >>> comes to > >>> > label > >>> > > management? I think that is confusing (besides earning Robert > >>> a lot of > >>> > > extra github notification mail thanks to participation on > >>> every PR :) > >>> > > > >>> > > Overall flinkbot is very useful, thanks for all the work on > >>> it! I heard > >>> > > positive feedback from other contributors, I think they see > their > >>> > > contributions are better received now. > >>> > > > >>> > > Thomas > >>> > > > >>> > > > >>> > > > >>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > >>> <[hidden email] <mailto:[hidden email]>> > >>> > wrote: > >>> > > > >>> > > > I will update labels only based on committer's approvals > (for > >>> > > everything), > >>> > > > I think that's cleaner. > >>> > > > > >>> > > > We can always revisit this. > >>> > > > > >>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > >>> <[hidden email] <mailto:[hidden email]>> > >>> > > > wrote: > >>> > > > > >>> > > > > Fore code-quality/description I agree, but consensus and > >>> the final > >>> > > > > approval should require a committer IMO. > >>> > > > > > >>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: > >>> > > > > > >>> > > > > I did not put any restrictions on who can communicate with > >>> the bot! > >>> > > > > But since there is currently no way of overriding > >>> somebody's approval > >>> > > for > >>> > > > > something, this can easily lead to such a situation. > >>> > > > > > >>> > > > > My thinking was that a committer still needs to manually > >>> check who > >>> > > > > approved a pull request, and I wanted to be open for > >>> non-committers > >>> > to > >>> > > > > participate in the review process. > >>> > > > > WIth the labels in place, this can easily send the wrong > >>> message. > >>> > > > > > >>> > > > > What should we do? > >>> > > > > A) we restrict sending commands to the bot to committers? > >>> > > > > B) only approvals from committers matter for applying > labels? > >>> > > > > C) we allow committers to override approvals > >>> > > > > > >>> > > > > I'm leaning towards B, as it encourages non-committers to > >>> > participate. > >>> > > > > > >>> > > > > > >>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler > >>> <[hidden email] <mailto:[hidden email]> > >>> > > > >>> > > > > wrote: > >>> > > > > > >>> > > > >> Just noticed that _anyone_ can approve a PR now, see > >>> > > > >> https://github.com/apache/flink/pull/7801. > >>> > > > >> > >>> > > > >> Not sure about the solution, but as it stands it is > >>> rather trivial > >>> > to > >>> > > > >> nuke the review process of the entire project. > >>> > > > >> > >>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > >>> > > > >> > Hey all, > >>> > > > >> > > >>> > > > >> > the flinkbot has been active for a week now, and I hope > >> the > >>> > initial > >>> > > > >> hiccups > >>> > > > >> > have been resolved :) > >>> > > > >> > > >>> > > > >> > I wanted to start this as a permanent thread to discuss > >>> problems > >>> > and > >>> > > > >> > improvements with the bot. > >>> > > > >> > > >>> > > > >> > *So please post here if you have questions, problems or > >>> ideas how > >>> > to > >>> > > > >> > improve it!* > >>> > > > >> > > >>> > > > >> > >>> > > > >> > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >> > > |
Re: [DISCUSS] Improve the flinkbot
|
|
How do you intend to keep the label up-to-date with whatever
modifications are made in JIRA? On 07.03.2019 13:40, Robert Metzger wrote: > I will automatically assign the Jira component as a label to the PR, yes. > You won't have to manually update the label on the PR, this will be done > automatically. > > So JIRA will stay the ground truth for setting the component correctly. > > On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler <[hidden email]> wrote: > >> Component labels seem a bit redundant. Every JIRA with an open PR >> already has a "pull-request-available" tag. >> So this information already exists. >> >> I assume you'll base the labels on the component tags at the time the PR >> is opened, but this also implies that they may be set incorrectly (or >> not at all) by the contributor. In this case we now have to update the >> component both in JIRA and on GitHub, and I'm most certainly not looking >> forward to that. >> >> On 06.03.2019 13:51, Robert Metzger wrote: >>> This is the picture: >>> >> https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png >>> Speaking about feature requests, priorities and time-spend: My plan was >> to >>> now work on introducing a new label category for the components. >>> This should get us a lot better overview over the per-component >>> status/health of pull requests. >>> >>> >>> On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler <[hidden email]> >> wrote: >>>> The image didn't go through. >>>> >>>> I would keep it as is; imo there are significantly more important things >>>> that I'd like Robert to spend time on. (literally everything in the >>>> Feature requests section) >>>> >>>> If we want to better distinguish new PRs I would suggest to either a) >>>> introduce a dedicated "New" label or b) not attach any label by default, >>>> and only attach the description label if someone has >>>> approved/disapproved it. >>>> >>>> On 06.03.2019 12:37, Robert Metzger wrote: >>>>> Hey Kurt, >>>>> thanks a lot for this idea. >>>>> >>>>> My reasoning behind using just one color is the following: I wanted to >>>>> use one color per category of labels. >>>>> So when we are introducing labels for components, that it'll look like >>>>> this: >>>>> >>>>> image.png >>>>> >>>>> But we could of course also go with color families per category. So >>>>> "review" is green colors, "component" is red colors and so on. >>>>> >>>>> If nobody objects (or agrees) with me, I'll change the colors soon. >>>>> >>>>> >>>>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] >>>>> <mailto:[hidden email]>> wrote: >>>>> >>>>> Hi Dev, >>>>> >>>>> I've been using the flinkbot and the label for a couple days, it >>>>> worked >>>>> really well! I have a minor suggestion, can we >>>>> use different colors for different labels? We don't need to have >>>>> different >>>>> colors for every label, but only to distinguish whether >>>>> someone had review the PR. >>>>> For example, "review=description?" is the initial default label, >>>>> and it may >>>>> indicate that no reviewer has been try to review it. >>>>> >>>>> For "review=architecture?", "review=consensus?", >>>>> "review=quality?", they >>>>> indicate that at least someone has try to review it and >>>>> approved something. It sounds like the review is in progress. >>>>> >>>>> For "review=approved ✅", it indicates the review is finished. >>>>> >>>>> So i think 3 colors is enough, it tell committers whether the >>>>> review has >>>>> not started yes, or in progress, or is finished. >>>>> >>>>> What do you think? >>>>> >>>>> Best, >>>>> Kurt >>>>> >>>>> >>>>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < >> [hidden email] >>>>> <mailto:[hidden email]>> wrote: >>>>> >>>>> > GitHub has two methods for authentication with the APIs: >>>>> > a) using an account's oauth token >>>>> > b) using the GitHub Apps API >>>>> > >>>>> > Most of the libraries for the GH API use a), so does Flinkbot. >>>>> The problem >>>>> > with a) is that it does not allow for fine-grained access >>>>> control, and >>>>> > Infra does not want to give Flinkbot "write" access to >>>>> "apache/flink". >>>>> > That's why I need to rewrite parts of the bot to support b), >>>>> which allows >>>>> > to give access only a repo's metadata, but not the code itself. >>>>> > >>>>> > >>>>> > >>>>> > >>>>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email] >>>>> <mailto:[hidden email]>> wrote: >>>>> > >>>>> > > It would be good to encourage participation of non-committers >>>>> in the >>>>> > review >>>>> > > process, so +1 for allowing everyone to operate the bot. >>>>> > > >>>>> > > Github approval will show a green checkmark for committer >>>> approval >>>>> > > (assuming accounts were linked via gitbox) - that should >> provide >>>>> > sufficient >>>>> > > orientation? >>>>> > > >>>>> > > I just noticed that flinkbot seems to act as Robert when it >>>>> comes to >>>>> > label >>>>> > > management? I think that is confusing (besides earning Robert >>>>> a lot of >>>>> > > extra github notification mail thanks to participation on >>>>> every PR :) >>>>> > > >>>>> > > Overall flinkbot is very useful, thanks for all the work on >>>>> it! I heard >>>>> > > positive feedback from other contributors, I think they see >> their >>>>> > > contributions are better received now. >>>>> > > >>>>> > > Thomas >>>>> > > >>>>> > > >>>>> > > >>>>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger >>>>> <[hidden email] <mailto:[hidden email]>> >>>>> > wrote: >>>>> > > >>>>> > > > I will update labels only based on committer's approvals >> (for >>>>> > > everything), >>>>> > > > I think that's cleaner. >>>>> > > > >>>>> > > > We can always revisit this. >>>>> > > > >>>>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler >>>>> <[hidden email] <mailto:[hidden email]>> >>>>> > > > wrote: >>>>> > > > >>>>> > > > > Fore code-quality/description I agree, but consensus and >>>>> the final >>>>> > > > > approval should require a committer IMO. >>>>> > > > > >>>>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: >>>>> > > > > >>>>> > > > > I did not put any restrictions on who can communicate with >>>>> the bot! >>>>> > > > > But since there is currently no way of overriding >>>>> somebody's approval >>>>> > > for >>>>> > > > > something, this can easily lead to such a situation. >>>>> > > > > >>>>> > > > > My thinking was that a committer still needs to manually >>>>> check who >>>>> > > > > approved a pull request, and I wanted to be open for >>>>> non-committers >>>>> > to >>>>> > > > > participate in the review process. >>>>> > > > > WIth the labels in place, this can easily send the wrong >>>>> message. >>>>> > > > > >>>>> > > > > What should we do? >>>>> > > > > A) we restrict sending commands to the bot to committers? >>>>> > > > > B) only approvals from committers matter for applying >> labels? >>>>> > > > > C) we allow committers to override approvals >>>>> > > > > >>>>> > > > > I'm leaning towards B, as it encourages non-committers to >>>>> > participate. >>>>> > > > > >>>>> > > > > >>>>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler >>>>> <[hidden email] <mailto:[hidden email]> >>>>> > > >>>>> > > > > wrote: >>>>> > > > > >>>>> > > > >> Just noticed that _anyone_ can approve a PR now, see >>>>> > > > >> https://github.com/apache/flink/pull/7801. >>>>> > > > >> >>>>> > > > >> Not sure about the solution, but as it stands it is >>>>> rather trivial >>>>> > to >>>>> > > > >> nuke the review process of the entire project. >>>>> > > > >> >>>>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: >>>>> > > > >> > Hey all, >>>>> > > > >> > >>>>> > > > >> > the flinkbot has been active for a week now, and I hope >>>> the >>>>> > initial >>>>> > > > >> hiccups >>>>> > > > >> > have been resolved :) >>>>> > > > >> > >>>>> > > > >> > I wanted to start this as a permanent thread to discuss >>>>> problems >>>>> > and >>>>> > > > >> > improvements with the bot. >>>>> > > > >> > >>>>> > > > >> > *So please post here if you have questions, problems or >>>>> ideas how >>>>> > to >>>>> > > > >> > improve it!* >>>>> > > > >> > >>>>> > > > >> >>>>> > > > >> >>>>> > > > > >>>>> > > > >>>>> > > >>>>> > >>>>> >> |
Re: [DISCUSS] Improve the flinkbot
|
|
Each Jira ticket has a "last updated" field, and in a JIRA search, you can
sort results by that field. So I will regularly check all Jira tickets which have been updated since the last time my tool checked. For all changed Jira tickets, I'll update the PR if the component has changed. The implementation will be a bit differently, to not run into rate limits with the JIRA or GitHub API. On Thu, Mar 7, 2019 at 2:40 PM Chesnay Schepler <[hidden email]> wrote: > How do you intend to keep the label up-to-date with whatever > modifications are made in JIRA? > > On 07.03.2019 13:40, Robert Metzger wrote: > > I will automatically assign the Jira component as a label to the PR, yes. > > You won't have to manually update the label on the PR, this will be done > > automatically. > > > > So JIRA will stay the ground truth for setting the component correctly. > > > > On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler <[hidden email]> > wrote: > > > >> Component labels seem a bit redundant. Every JIRA with an open PR > >> already has a "pull-request-available" tag. > >> So this information already exists. > >> > >> I assume you'll base the labels on the component tags at the time the PR > >> is opened, but this also implies that they may be set incorrectly (or > >> not at all) by the contributor. In this case we now have to update the > >> component both in JIRA and on GitHub, and I'm most certainly not looking > >> forward to that. > >> > >> On 06.03.2019 13:51, Robert Metzger wrote: > >>> This is the picture: > >>> > >> > https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png > >>> Speaking about feature requests, priorities and time-spend: My plan was > >> to > >>> now work on introducing a new label category for the components. > >>> This should get us a lot better overview over the per-component > >>> status/health of pull requests. > >>> > >>> > >>> On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler <[hidden email]> > >> wrote: > >>>> The image didn't go through. > >>>> > >>>> I would keep it as is; imo there are significantly more important > things > >>>> that I'd like Robert to spend time on. (literally everything in the > >>>> Feature requests section) > >>>> > >>>> If we want to better distinguish new PRs I would suggest to either a) > >>>> introduce a dedicated "New" label or b) not attach any label by > default, > >>>> and only attach the description label if someone has > >>>> approved/disapproved it. > >>>> > >>>> On 06.03.2019 12:37, Robert Metzger wrote: > >>>>> Hey Kurt, > >>>>> thanks a lot for this idea. > >>>>> > >>>>> My reasoning behind using just one color is the following: I wanted > to > >>>>> use one color per category of labels. > >>>>> So when we are introducing labels for components, that it'll look > like > >>>>> this: > >>>>> > >>>>> image.png > >>>>> > >>>>> But we could of course also go with color families per category. So > >>>>> "review" is green colors, "component" is red colors and so on. > >>>>> > >>>>> If nobody objects (or agrees) with me, I'll change the colors soon. > >>>>> > >>>>> > >>>>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > >>>>> <mailto:[hidden email]>> wrote: > >>>>> > >>>>> Hi Dev, > >>>>> > >>>>> I've been using the flinkbot and the label for a couple days, > it > >>>>> worked > >>>>> really well! I have a minor suggestion, can we > >>>>> use different colors for different labels? We don't need to > have > >>>>> different > >>>>> colors for every label, but only to distinguish whether > >>>>> someone had review the PR. > >>>>> For example, "review=description?" is the initial default > label, > >>>>> and it may > >>>>> indicate that no reviewer has been try to review it. > >>>>> > >>>>> For "review=architecture?", "review=consensus?", > >>>>> "review=quality?", they > >>>>> indicate that at least someone has try to review it and > >>>>> approved something. It sounds like the review is in progress. > >>>>> > >>>>> For "review=approved ✅", it indicates the review is finished. > >>>>> > >>>>> So i think 3 colors is enough, it tell committers whether the > >>>>> review has > >>>>> not started yes, or in progress, or is finished. > >>>>> > >>>>> What do you think? > >>>>> > >>>>> Best, > >>>>> Kurt > >>>>> > >>>>> > >>>>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < > >> [hidden email] > >>>>> <mailto:[hidden email]>> wrote: > >>>>> > >>>>> > GitHub has two methods for authentication with the APIs: > >>>>> > a) using an account's oauth token > >>>>> > b) using the GitHub Apps API > >>>>> > > >>>>> > Most of the libraries for the GH API use a), so does > Flinkbot. > >>>>> The problem > >>>>> > with a) is that it does not allow for fine-grained access > >>>>> control, and > >>>>> > Infra does not want to give Flinkbot "write" access to > >>>>> "apache/flink". > >>>>> > That's why I need to rewrite parts of the bot to support b), > >>>>> which allows > >>>>> > to give access only a repo's metadata, but not the code > itself. > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise <[hidden email] > >>>>> <mailto:[hidden email]>> wrote: > >>>>> > > >>>>> > > It would be good to encourage participation of > non-committers > >>>>> in the > >>>>> > review > >>>>> > > process, so +1 for allowing everyone to operate the bot. > >>>>> > > > >>>>> > > Github approval will show a green checkmark for committer > >>>> approval > >>>>> > > (assuming accounts were linked via gitbox) - that should > >> provide > >>>>> > sufficient > >>>>> > > orientation? > >>>>> > > > >>>>> > > I just noticed that flinkbot seems to act as Robert when it > >>>>> comes to > >>>>> > label > >>>>> > > management? I think that is confusing (besides earning > Robert > >>>>> a lot of > >>>>> > > extra github notification mail thanks to participation on > >>>>> every PR :) > >>>>> > > > >>>>> > > Overall flinkbot is very useful, thanks for all the work on > >>>>> it! I heard > >>>>> > > positive feedback from other contributors, I think they see > >> their > >>>>> > > contributions are better received now. > >>>>> > > > >>>>> > > Thomas > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > >>>>> <[hidden email] <mailto:[hidden email]>> > >>>>> > wrote: > >>>>> > > > >>>>> > > > I will update labels only based on committer's approvals > >> (for > >>>>> > > everything), > >>>>> > > > I think that's cleaner. > >>>>> > > > > >>>>> > > > We can always revisit this. > >>>>> > > > > >>>>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > >>>>> <[hidden email] <mailto:[hidden email]>> > >>>>> > > > wrote: > >>>>> > > > > >>>>> > > > > Fore code-quality/description I agree, but consensus > and > >>>>> the final > >>>>> > > > > approval should require a committer IMO. > >>>>> > > > > > >>>>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: > >>>>> > > > > > >>>>> > > > > I did not put any restrictions on who can communicate > with > >>>>> the bot! > >>>>> > > > > But since there is currently no way of overriding > >>>>> somebody's approval > >>>>> > > for > >>>>> > > > > something, this can easily lead to such a situation. > >>>>> > > > > > >>>>> > > > > My thinking was that a committer still needs to > manually > >>>>> check who > >>>>> > > > > approved a pull request, and I wanted to be open for > >>>>> non-committers > >>>>> > to > >>>>> > > > > participate in the review process. > >>>>> > > > > WIth the labels in place, this can easily send the > wrong > >>>>> message. > >>>>> > > > > > >>>>> > > > > What should we do? > >>>>> > > > > A) we restrict sending commands to the bot to > committers? > >>>>> > > > > B) only approvals from committers matter for applying > >> labels? > >>>>> > > > > C) we allow committers to override approvals > >>>>> > > > > > >>>>> > > > > I'm leaning towards B, as it encourages non-committers > to > >>>>> > participate. > >>>>> > > > > > >>>>> > > > > > >>>>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler > >>>>> <[hidden email] <mailto:[hidden email]> > >>>>> > > > >>>>> > > > > wrote: > >>>>> > > > > > >>>>> > > > >> Just noticed that _anyone_ can approve a PR now, see > >>>>> > > > >> https://github.com/apache/flink/pull/7801. > >>>>> > > > >> > >>>>> > > > >> Not sure about the solution, but as it stands it is > >>>>> rather trivial > >>>>> > to > >>>>> > > > >> nuke the review process of the entire project. > >>>>> > > > >> > >>>>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > >>>>> > > > >> > Hey all, > >>>>> > > > >> > > >>>>> > > > >> > the flinkbot has been active for a week now, and I > hope > >>>> the > >>>>> > initial > >>>>> > > > >> hiccups > >>>>> > > > >> > have been resolved :) > >>>>> > > > >> > > >>>>> > > > >> > I wanted to start this as a permanent thread to > discuss > >>>>> problems > >>>>> > and > >>>>> > > > >> > improvements with the bot. > >>>>> > > > >> > > >>>>> > > > >> > *So please post here if you have questions, > problems or > >>>>> ideas how > >>>>> > to > >>>>> > > > >> > improve it!* > >>>>> > > > >> > > >>>>> > > > >> > >>>>> > > > >> > >>>>> > > > > > >>>>> > > > > >>>>> > > > >>>>> > > >>>>> > >> > > |
|
|
Hi all,
In the past discussion of Supporting Chinese Documentation for Apache Flink[1], we reach a consensus to add a documentation check item to the flinkbot review process. I propose the idea here to get some more feedbacks about this. The new item we want to add is: ``` ### 6. Are English and Chinese documentation updated? If the pull request introduces a new feature, the feature should be documented. The Flink community is maintaining both English and Chinese documents. So both English and Chinese documentation should be updated. If you are not familiar with Chinese language, please open a JIRA tagged with the `chinese-translation` component for Chinese documentation translation and link it with current JIRA issue. If you are familiar with Chinese language, you are encouraged to update both sides in one pull request. ``` We have opened a pull request [2] to update it to the website. What do you think about this? Thanks, Jark [1] http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Contributing-Chinese-website-and-docs-to-Apache-Flink-tt26603.html#a26890 [2] https://github.com/apache/flink-web/pull/190 On Thu, 7 Mar 2019 at 21:48, Robert Metzger <[hidden email]> wrote: > Each Jira ticket has a "last updated" field, and in a JIRA search, you can > sort results by that field. > > So I will regularly check all Jira tickets which have been updated since > the last time my tool checked. For all changed Jira tickets, I'll update > the PR if the component has changed. > > The implementation will be a bit differently, to not run into rate limits > with the JIRA or GitHub API. > > On Thu, Mar 7, 2019 at 2:40 PM Chesnay Schepler <[hidden email]> > wrote: > > > How do you intend to keep the label up-to-date with whatever > > modifications are made in JIRA? > > > > On 07.03.2019 13:40, Robert Metzger wrote: > > > I will automatically assign the Jira component as a label to the PR, > yes. > > > You won't have to manually update the label on the PR, this will be > done > > > automatically. > > > > > > So JIRA will stay the ground truth for setting the component correctly. > > > > > > On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler <[hidden email]> > > wrote: > > > > > >> Component labels seem a bit redundant. Every JIRA with an open PR > > >> already has a "pull-request-available" tag. > > >> So this information already exists. > > >> > > >> I assume you'll base the labels on the component tags at the time the > PR > > >> is opened, but this also implies that they may be set incorrectly (or > > >> not at all) by the contributor. In this case we now have to update the > > >> component both in JIRA and on GitHub, and I'm most certainly not > looking > > >> forward to that. > > >> > > >> On 06.03.2019 13:51, Robert Metzger wrote: > > >>> This is the picture: > > >>> > > >> > > > https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png > > >>> Speaking about feature requests, priorities and time-spend: My plan > was > > >> to > > >>> now work on introducing a new label category for the components. > > >>> This should get us a lot better overview over the per-component > > >>> status/health of pull requests. > > >>> > > >>> > > >>> On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler <[hidden email] > > > > >> wrote: > > >>>> The image didn't go through. > > >>>> > > >>>> I would keep it as is; imo there are significantly more important > > things > > >>>> that I'd like Robert to spend time on. (literally everything in the > > >>>> Feature requests section) > > >>>> > > >>>> If we want to better distinguish new PRs I would suggest to either > a) > > >>>> introduce a dedicated "New" label or b) not attach any label by > > default, > > >>>> and only attach the description label if someone has > > >>>> approved/disapproved it. > > >>>> > > >>>> On 06.03.2019 12:37, Robert Metzger wrote: > > >>>>> Hey Kurt, > > >>>>> thanks a lot for this idea. > > >>>>> > > >>>>> My reasoning behind using just one color is the following: I wanted > > to > > >>>>> use one color per category of labels. > > >>>>> So when we are introducing labels for components, that it'll look > > like > > >>>>> this: > > >>>>> > > >>>>> image.png > > >>>>> > > >>>>> But we could of course also go with color families per category. So > > >>>>> "review" is green colors, "component" is red colors and so on. > > >>>>> > > >>>>> If nobody objects (or agrees) with me, I'll change the colors soon. > > >>>>> > > >>>>> > > >>>>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > > >>>>> <mailto:[hidden email]>> wrote: > > >>>>> > > >>>>> Hi Dev, > > >>>>> > > >>>>> I've been using the flinkbot and the label for a couple days, > > it > > >>>>> worked > > >>>>> really well! I have a minor suggestion, can we > > >>>>> use different colors for different labels? We don't need to > > have > > >>>>> different > > >>>>> colors for every label, but only to distinguish whether > > >>>>> someone had review the PR. > > >>>>> For example, "review=description?" is the initial default > > label, > > >>>>> and it may > > >>>>> indicate that no reviewer has been try to review it. > > >>>>> > > >>>>> For "review=architecture?", "review=consensus?", > > >>>>> "review=quality?", they > > >>>>> indicate that at least someone has try to review it and > > >>>>> approved something. It sounds like the review is in progress. > > >>>>> > > >>>>> For "review=approved ✅", it indicates the review is finished. > > >>>>> > > >>>>> So i think 3 colors is enough, it tell committers whether the > > >>>>> review has > > >>>>> not started yes, or in progress, or is finished. > > >>>>> > > >>>>> What do you think? > > >>>>> > > >>>>> Best, > > >>>>> Kurt > > >>>>> > > >>>>> > > >>>>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < > > >> [hidden email] > > >>>>> <mailto:[hidden email]>> wrote: > > >>>>> > > >>>>> > GitHub has two methods for authentication with the APIs: > > >>>>> > a) using an account's oauth token > > >>>>> > b) using the GitHub Apps API > > >>>>> > > > >>>>> > Most of the libraries for the GH API use a), so does > > Flinkbot. > > >>>>> The problem > > >>>>> > with a) is that it does not allow for fine-grained access > > >>>>> control, and > > >>>>> > Infra does not want to give Flinkbot "write" access to > > >>>>> "apache/flink". > > >>>>> > That's why I need to rewrite parts of the bot to support > b), > > >>>>> which allows > > >>>>> > to give access only a repo's metadata, but not the code > > itself. > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise < > [hidden email] > > >>>>> <mailto:[hidden email]>> wrote: > > >>>>> > > > >>>>> > > It would be good to encourage participation of > > non-committers > > >>>>> in the > > >>>>> > review > > >>>>> > > process, so +1 for allowing everyone to operate the bot. > > >>>>> > > > > >>>>> > > Github approval will show a green checkmark for committer > > >>>> approval > > >>>>> > > (assuming accounts were linked via gitbox) - that should > > >> provide > > >>>>> > sufficient > > >>>>> > > orientation? > > >>>>> > > > > >>>>> > > I just noticed that flinkbot seems to act as Robert when > it > > >>>>> comes to > > >>>>> > label > > >>>>> > > management? I think that is confusing (besides earning > > Robert > > >>>>> a lot of > > >>>>> > > extra github notification mail thanks to participation on > > >>>>> every PR :) > > >>>>> > > > > >>>>> > > Overall flinkbot is very useful, thanks for all the work > on > > >>>>> it! I heard > > >>>>> > > positive feedback from other contributors, I think they > see > > >> their > > >>>>> > > contributions are better received now. > > >>>>> > > > > >>>>> > > Thomas > > >>>>> > > > > >>>>> > > > > >>>>> > > > > >>>>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > > >>>>> <[hidden email] <mailto:[hidden email]>> > > >>>>> > wrote: > > >>>>> > > > > >>>>> > > > I will update labels only based on committer's > approvals > > >> (for > > >>>>> > > everything), > > >>>>> > > > I think that's cleaner. > > >>>>> > > > > > >>>>> > > > We can always revisit this. > > >>>>> > > > > > >>>>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > > >>>>> <[hidden email] <mailto:[hidden email]>> > > >>>>> > > > wrote: > > >>>>> > > > > > >>>>> > > > > Fore code-quality/description I agree, but consensus > > and > > >>>>> the final > > >>>>> > > > > approval should require a committer IMO. > > >>>>> > > > > > > >>>>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > >>>>> > > > > > > >>>>> > > > > I did not put any restrictions on who can communicate > > with > > >>>>> the bot! > > >>>>> > > > > But since there is currently no way of overriding > > >>>>> somebody's approval > > >>>>> > > for > > >>>>> > > > > something, this can easily lead to such a situation. > > >>>>> > > > > > > >>>>> > > > > My thinking was that a committer still needs to > > manually > > >>>>> check who > > >>>>> > > > > approved a pull request, and I wanted to be open for > > >>>>> non-committers > > >>>>> > to > > >>>>> > > > > participate in the review process. > > >>>>> > > > > WIth the labels in place, this can easily send the > > wrong > > >>>>> message. > > >>>>> > > > > > > >>>>> > > > > What should we do? > > >>>>> > > > > A) we restrict sending commands to the bot to > > committers? > > >>>>> > > > > B) only approvals from committers matter for applying > > >> labels? > > >>>>> > > > > C) we allow committers to override approvals > > >>>>> > > > > > > >>>>> > > > > I'm leaning towards B, as it encourages > non-committers > > to > > >>>>> > participate. > > >>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler > > >>>>> <[hidden email] <mailto:[hidden email]> > > >>>>> > > > > >>>>> > > > > wrote: > > >>>>> > > > > > > >>>>> > > > >> Just noticed that _anyone_ can approve a PR now, see > > >>>>> > > > >> https://github.com/apache/flink/pull/7801. > > >>>>> > > > >> > > >>>>> > > > >> Not sure about the solution, but as it stands it is > > >>>>> rather trivial > > >>>>> > to > > >>>>> > > > >> nuke the review process of the entire project. > > >>>>> > > > >> > > >>>>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > >>>>> > > > >> > Hey all, > > >>>>> > > > >> > > > >>>>> > > > >> > the flinkbot has been active for a week now, and I > > hope > > >>>> the > > >>>>> > initial > > >>>>> > > > >> hiccups > > >>>>> > > > >> > have been resolved :) > > >>>>> > > > >> > > > >>>>> > > > >> > I wanted to start this as a permanent thread to > > discuss > > >>>>> problems > > >>>>> > and > > >>>>> > > > >> > improvements with the bot. > > >>>>> > > > >> > > > >>>>> > > > >> > *So please post here if you have questions, > > problems or > > >>>>> ideas how > > >>>>> > to > > >>>>> > > > >> > improve it!* > > >>>>> > > > >> > > > >>>>> > > > >> > > >>>>> > > > >> > > >>>>> > > > > > > >>>>> > > > > > >>>>> > > > > >>>>> > > > >>>>> > > >> > > > > > |
Re: [DISCUSS] Improve the flinkbot
|
|
Hi Jark,
Thanks for driving the effort to integrate the Chinese website! We have the policy that new features / improvements should be documented in the same PR for a long time. So far, this was checked by reviewers and committers but often overlooked or decided to add documentation in a subsequent PR. When we introduced the PR template, we added "Documentation" as a dedicated section to remind contributors (and reviewers) about the documentation policy. I think adding an additional step in the review process to check the documentation would help to enforce the policy. +1 for this proposal. IMO, this is independent of the Chinese documentation, but it would certainly help to keep both versions in sync. Best, Fabian Am Do., 21. März 2019 um 06:10 Uhr schrieb Jark Wu <[hidden email]>: > Hi all, > > In the past discussion of Supporting Chinese Documentation for Apache > Flink[1], we reach a consensus to add a documentation check item to the > flinkbot review process. > > I propose the idea here to get some more feedbacks about this. > > The new item we want to add is: > > ``` > ### 6. Are English and Chinese documentation updated? > > If the pull request introduces a new feature, the feature should be > documented. The Flink community is maintaining both English and Chinese > documents. So both English and Chinese documentation should be updated. If > you are not familiar with Chinese language, please open a JIRA tagged with > the `chinese-translation` component for Chinese documentation translation > and link it with current JIRA issue. If you are familiar with Chinese > language, you are encouraged to update both sides in one pull request. > ``` > > We have opened a pull request [2] to update it to the website. > > What do you think about this? > > Thanks, > Jark > > > [1] > > http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Contributing-Chinese-website-and-docs-to-Apache-Flink-tt26603.html#a26890 > [2] https://github.com/apache/flink-web/pull/190 > > On Thu, 7 Mar 2019 at 21:48, Robert Metzger <[hidden email]> wrote: > > > Each Jira ticket has a "last updated" field, and in a JIRA search, you > can > > sort results by that field. > > > > So I will regularly check all Jira tickets which have been updated since > > the last time my tool checked. For all changed Jira tickets, I'll update > > the PR if the component has changed. > > > > The implementation will be a bit differently, to not run into rate limits > > with the JIRA or GitHub API. > > > > On Thu, Mar 7, 2019 at 2:40 PM Chesnay Schepler <[hidden email]> > > wrote: > > > > > How do you intend to keep the label up-to-date with whatever > > > modifications are made in JIRA? > > > > > > On 07.03.2019 13:40, Robert Metzger wrote: > > > > I will automatically assign the Jira component as a label to the PR, > > yes. > > > > You won't have to manually update the label on the PR, this will be > > done > > > > automatically. > > > > > > > > So JIRA will stay the ground truth for setting the component > correctly. > > > > > > > > On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler <[hidden email] > > > > > wrote: > > > > > > > >> Component labels seem a bit redundant. Every JIRA with an open PR > > > >> already has a "pull-request-available" tag. > > > >> So this information already exists. > > > >> > > > >> I assume you'll base the labels on the component tags at the time > the > > PR > > > >> is opened, but this also implies that they may be set incorrectly > (or > > > >> not at all) by the contributor. In this case we now have to update > the > > > >> component both in JIRA and on GitHub, and I'm most certainly not > > looking > > > >> forward to that. > > > >> > > > >> On 06.03.2019 13:51, Robert Metzger wrote: > > > >>> This is the picture: > > > >>> > > > >> > > > > > > https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png > > > >>> Speaking about feature requests, priorities and time-spend: My plan > > was > > > >> to > > > >>> now work on introducing a new label category for the components. > > > >>> This should get us a lot better overview over the per-component > > > >>> status/health of pull requests. > > > >>> > > > >>> > > > >>> On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler < > [hidden email] > > > > > > >> wrote: > > > >>>> The image didn't go through. > > > >>>> > > > >>>> I would keep it as is; imo there are significantly more important > > > things > > > >>>> that I'd like Robert to spend time on. (literally everything in > the > > > >>>> Feature requests section) > > > >>>> > > > >>>> If we want to better distinguish new PRs I would suggest to either > > a) > > > >>>> introduce a dedicated "New" label or b) not attach any label by > > > default, > > > >>>> and only attach the description label if someone has > > > >>>> approved/disapproved it. > > > >>>> > > > >>>> On 06.03.2019 12:37, Robert Metzger wrote: > > > >>>>> Hey Kurt, > > > >>>>> thanks a lot for this idea. > > > >>>>> > > > >>>>> My reasoning behind using just one color is the following: I > wanted > > > to > > > >>>>> use one color per category of labels. > > > >>>>> So when we are introducing labels for components, that it'll look > > > like > > > >>>>> this: > > > >>>>> > > > >>>>> image.png > > > >>>>> > > > >>>>> But we could of course also go with color families per category. > So > > > >>>>> "review" is green colors, "component" is red colors and so on. > > > >>>>> > > > >>>>> If nobody objects (or agrees) with me, I'll change the colors > soon. > > > >>>>> > > > >>>>> > > > >>>>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > > > >>>>> <mailto:[hidden email]>> wrote: > > > >>>>> > > > >>>>> Hi Dev, > > > >>>>> > > > >>>>> I've been using the flinkbot and the label for a couple > days, > > > it > > > >>>>> worked > > > >>>>> really well! I have a minor suggestion, can we > > > >>>>> use different colors for different labels? We don't need to > > > have > > > >>>>> different > > > >>>>> colors for every label, but only to distinguish whether > > > >>>>> someone had review the PR. > > > >>>>> For example, "review=description?" is the initial default > > > label, > > > >>>>> and it may > > > >>>>> indicate that no reviewer has been try to review it. > > > >>>>> > > > >>>>> For "review=architecture?", "review=consensus?", > > > >>>>> "review=quality?", they > > > >>>>> indicate that at least someone has try to review it and > > > >>>>> approved something. It sounds like the review is in > progress. > > > >>>>> > > > >>>>> For "review=approved ✅", it indicates the review is > finished. > > > >>>>> > > > >>>>> So i think 3 colors is enough, it tell committers whether > the > > > >>>>> review has > > > >>>>> not started yes, or in progress, or is finished. > > > >>>>> > > > >>>>> What do you think? > > > >>>>> > > > >>>>> Best, > > > >>>>> Kurt > > > >>>>> > > > >>>>> > > > >>>>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < > > > >> [hidden email] > > > >>>>> <mailto:[hidden email]>> wrote: > > > >>>>> > > > >>>>> > GitHub has two methods for authentication with the APIs: > > > >>>>> > a) using an account's oauth token > > > >>>>> > b) using the GitHub Apps API > > > >>>>> > > > > >>>>> > Most of the libraries for the GH API use a), so does > > > Flinkbot. > > > >>>>> The problem > > > >>>>> > with a) is that it does not allow for fine-grained access > > > >>>>> control, and > > > >>>>> > Infra does not want to give Flinkbot "write" access to > > > >>>>> "apache/flink". > > > >>>>> > That's why I need to rewrite parts of the bot to support > > b), > > > >>>>> which allows > > > >>>>> > to give access only a repo's metadata, but not the code > > > itself. > > > >>>>> > > > > >>>>> > > > > >>>>> > > > > >>>>> > > > > >>>>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise < > > [hidden email] > > > >>>>> <mailto:[hidden email]>> wrote: > > > >>>>> > > > > >>>>> > > It would be good to encourage participation of > > > non-committers > > > >>>>> in the > > > >>>>> > review > > > >>>>> > > process, so +1 for allowing everyone to operate the > bot. > > > >>>>> > > > > > >>>>> > > Github approval will show a green checkmark for > committer > > > >>>> approval > > > >>>>> > > (assuming accounts were linked via gitbox) - that > should > > > >> provide > > > >>>>> > sufficient > > > >>>>> > > orientation? > > > >>>>> > > > > > >>>>> > > I just noticed that flinkbot seems to act as Robert > when > > it > > > >>>>> comes to > > > >>>>> > label > > > >>>>> > > management? I think that is confusing (besides earning > > > Robert > > > >>>>> a lot of > > > >>>>> > > extra github notification mail thanks to participation > on > > > >>>>> every PR :) > > > >>>>> > > > > > >>>>> > > Overall flinkbot is very useful, thanks for all the > work > > on > > > >>>>> it! I heard > > > >>>>> > > positive feedback from other contributors, I think they > > see > > > >> their > > > >>>>> > > contributions are better received now. > > > >>>>> > > > > > >>>>> > > Thomas > > > >>>>> > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > > > >>>>> <[hidden email] <mailto:[hidden email]>> > > > >>>>> > wrote: > > > >>>>> > > > > > >>>>> > > > I will update labels only based on committer's > > approvals > > > >> (for > > > >>>>> > > everything), > > > >>>>> > > > I think that's cleaner. > > > >>>>> > > > > > > >>>>> > > > We can always revisit this. > > > >>>>> > > > > > > >>>>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > > > >>>>> <[hidden email] <mailto:[hidden email]>> > > > >>>>> > > > wrote: > > > >>>>> > > > > > > >>>>> > > > > Fore code-quality/description I agree, but > consensus > > > and > > > >>>>> the final > > > >>>>> > > > > approval should require a committer IMO. > > > >>>>> > > > > > > > >>>>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > >>>>> > > > > > > > >>>>> > > > > I did not put any restrictions on who can > communicate > > > with > > > >>>>> the bot! > > > >>>>> > > > > But since there is currently no way of overriding > > > >>>>> somebody's approval > > > >>>>> > > for > > > >>>>> > > > > something, this can easily lead to such a > situation. > > > >>>>> > > > > > > > >>>>> > > > > My thinking was that a committer still needs to > > > manually > > > >>>>> check who > > > >>>>> > > > > approved a pull request, and I wanted to be open > for > > > >>>>> non-committers > > > >>>>> > to > > > >>>>> > > > > participate in the review process. > > > >>>>> > > > > WIth the labels in place, this can easily send the > > > wrong > > > >>>>> message. > > > >>>>> > > > > > > > >>>>> > > > > What should we do? > > > >>>>> > > > > A) we restrict sending commands to the bot to > > > committers? > > > >>>>> > > > > B) only approvals from committers matter for > applying > > > >> labels? > > > >>>>> > > > > C) we allow committers to override approvals > > > >>>>> > > > > > > > >>>>> > > > > I'm leaning towards B, as it encourages > > non-committers > > > to > > > >>>>> > participate. > > > >>>>> > > > > > > > >>>>> > > > > > > > >>>>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler > > > >>>>> <[hidden email] <mailto:[hidden email]> > > > >>>>> > > > > > >>>>> > > > > wrote: > > > >>>>> > > > > > > > >>>>> > > > >> Just noticed that _anyone_ can approve a PR now, > see > > > >>>>> > > > >> https://github.com/apache/flink/pull/7801. > > > >>>>> > > > >> > > > >>>>> > > > >> Not sure about the solution, but as it stands it > is > > > >>>>> rather trivial > > > >>>>> > to > > > >>>>> > > > >> nuke the review process of the entire project. > > > >>>>> > > > >> > > > >>>>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > > >>>>> > > > >> > Hey all, > > > >>>>> > > > >> > > > > >>>>> > > > >> > the flinkbot has been active for a week now, > and I > > > hope > > > >>>> the > > > >>>>> > initial > > > >>>>> > > > >> hiccups > > > >>>>> > > > >> > have been resolved :) > > > >>>>> > > > >> > > > > >>>>> > > > >> > I wanted to start this as a permanent thread to > > > discuss > > > >>>>> problems > > > >>>>> > and > > > >>>>> > > > >> > improvements with the bot. > > > >>>>> > > > >> > > > > >>>>> > > > >> > *So please post here if you have questions, > > > problems or > > > >>>>> ideas how > > > >>>>> > to > > > >>>>> > > > >> > improve it!* > > > >>>>> > > > >> > > > > >>>>> > > > >> > > > >>>>> > > > >> > > > >>>>> > > > > > > > >>>>> > > > > > > >>>>> > > > > > >>>>> > > > > >>>>> > > > >> > > > > > > > > > |
|
|
+1, sounds good, Jark.
On Fri, Mar 22, 2019 at 1:55 AM Fabian Hueske <[hidden email]> wrote: > Hi Jark, > > Thanks for driving the effort to integrate the Chinese website! > > We have the policy that new features / improvements should be documented in > the same PR for a long time. > So far, this was checked by reviewers and committers but often overlooked > or decided to add documentation in a subsequent PR. > When we introduced the PR template, we added "Documentation" as a dedicated > section to remind contributors (and reviewers) about the documentation > policy. > I think adding an additional step in the review process to check the > documentation would help to enforce the policy. > > +1 for this proposal. > IMO, this is independent of the Chinese documentation, but it would > certainly help to keep both versions in sync. > > Best, Fabian > > > Am Do., 21. März 2019 um 06:10 Uhr schrieb Jark Wu <[hidden email]>: > > > Hi all, > > > > In the past discussion of Supporting Chinese Documentation for Apache > > Flink[1], we reach a consensus to add a documentation check item to the > > flinkbot review process. > > > > I propose the idea here to get some more feedbacks about this. > > > > The new item we want to add is: > > > > ``` > > ### 6. Are English and Chinese documentation updated? > > > > If the pull request introduces a new feature, the feature should be > > documented. The Flink community is maintaining both English and Chinese > > documents. So both English and Chinese documentation should be updated. > If > > you are not familiar with Chinese language, please open a JIRA tagged > with > > the `chinese-translation` component for Chinese documentation translation > > and link it with current JIRA issue. If you are familiar with Chinese > > language, you are encouraged to update both sides in one pull request. > > ``` > > > > We have opened a pull request [2] to update it to the website. > > > > What do you think about this? > > > > Thanks, > > Jark > > > > > > [1] > > > > > http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Contributing-Chinese-website-and-docs-to-Apache-Flink-tt26603.html#a26890 > > [2] https://github.com/apache/flink-web/pull/190 > > > > On Thu, 7 Mar 2019 at 21:48, Robert Metzger <[hidden email]> wrote: > > > > > Each Jira ticket has a "last updated" field, and in a JIRA search, you > > can > > > sort results by that field. > > > > > > So I will regularly check all Jira tickets which have been updated > since > > > the last time my tool checked. For all changed Jira tickets, I'll > update > > > the PR if the component has changed. > > > > > > The implementation will be a bit differently, to not run into rate > limits > > > with the JIRA or GitHub API. > > > > > > On Thu, Mar 7, 2019 at 2:40 PM Chesnay Schepler <[hidden email]> > > > wrote: > > > > > > > How do you intend to keep the label up-to-date with whatever > > > > modifications are made in JIRA? > > > > > > > > On 07.03.2019 13:40, Robert Metzger wrote: > > > > > I will automatically assign the Jira component as a label to the > PR, > > > yes. > > > > > You won't have to manually update the label on the PR, this will be > > > done > > > > > automatically. > > > > > > > > > > So JIRA will stay the ground truth for setting the component > > correctly. > > > > > > > > > > On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler < > [hidden email] > > > > > > > wrote: > > > > > > > > > >> Component labels seem a bit redundant. Every JIRA with an open PR > > > > >> already has a "pull-request-available" tag. > > > > >> So this information already exists. > > > > >> > > > > >> I assume you'll base the labels on the component tags at the time > > the > > > PR > > > > >> is opened, but this also implies that they may be set incorrectly > > (or > > > > >> not at all) by the contributor. In this case we now have to update > > the > > > > >> component both in JIRA and on GitHub, and I'm most certainly not > > > looking > > > > >> forward to that. > > > > >> > > > > >> On 06.03.2019 13:51, Robert Metzger wrote: > > > > >>> This is the picture: > > > > >>> > > > > >> > > > > > > > > > > https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png > > > > >>> Speaking about feature requests, priorities and time-spend: My > plan > > > was > > > > >> to > > > > >>> now work on introducing a new label category for the components. > > > > >>> This should get us a lot better overview over the per-component > > > > >>> status/health of pull requests. > > > > >>> > > > > >>> > > > > >>> On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler < > > [hidden email] > > > > > > > > >> wrote: > > > > >>>> The image didn't go through. > > > > >>>> > > > > >>>> I would keep it as is; imo there are significantly more > important > > > > things > > > > >>>> that I'd like Robert to spend time on. (literally everything in > > the > > > > >>>> Feature requests section) > > > > >>>> > > > > >>>> If we want to better distinguish new PRs I would suggest to > either > > > a) > > > > >>>> introduce a dedicated "New" label or b) not attach any label by > > > > default, > > > > >>>> and only attach the description label if someone has > > > > >>>> approved/disapproved it. > > > > >>>> > > > > >>>> On 06.03.2019 12:37, Robert Metzger wrote: > > > > >>>>> Hey Kurt, > > > > >>>>> thanks a lot for this idea. > > > > >>>>> > > > > >>>>> My reasoning behind using just one color is the following: I > > wanted > > > > to > > > > >>>>> use one color per category of labels. > > > > >>>>> So when we are introducing labels for components, that it'll > look > > > > like > > > > >>>>> this: > > > > >>>>> > > > > >>>>> image.png > > > > >>>>> > > > > >>>>> But we could of course also go with color families per > category. > > So > > > > >>>>> "review" is green colors, "component" is red colors and so on. > > > > >>>>> > > > > >>>>> If nobody objects (or agrees) with me, I'll change the colors > > soon. > > > > >>>>> > > > > >>>>> > > > > >>>>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > > > > >>>>> <mailto:[hidden email]>> wrote: > > > > >>>>> > > > > >>>>> Hi Dev, > > > > >>>>> > > > > >>>>> I've been using the flinkbot and the label for a couple > > days, > > > > it > > > > >>>>> worked > > > > >>>>> really well! I have a minor suggestion, can we > > > > >>>>> use different colors for different labels? We don't need > to > > > > have > > > > >>>>> different > > > > >>>>> colors for every label, but only to distinguish whether > > > > >>>>> someone had review the PR. > > > > >>>>> For example, "review=description?" is the initial default > > > > label, > > > > >>>>> and it may > > > > >>>>> indicate that no reviewer has been try to review it. > > > > >>>>> > > > > >>>>> For "review=architecture?", "review=consensus?", > > > > >>>>> "review=quality?", they > > > > >>>>> indicate that at least someone has try to review it and > > > > >>>>> approved something. It sounds like the review is in > > progress. > > > > >>>>> > > > > >>>>> For "review=approved ✅", it indicates the review is > > finished. > > > > >>>>> > > > > >>>>> So i think 3 colors is enough, it tell committers whether > > the > > > > >>>>> review has > > > > >>>>> not started yes, or in progress, or is finished. > > > > >>>>> > > > > >>>>> What do you think? > > > > >>>>> > > > > >>>>> Best, > > > > >>>>> Kurt > > > > >>>>> > > > > >>>>> > > > > >>>>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < > > > > >> [hidden email] > > > > >>>>> <mailto:[hidden email]>> wrote: > > > > >>>>> > > > > >>>>> > GitHub has two methods for authentication with the > APIs: > > > > >>>>> > a) using an account's oauth token > > > > >>>>> > b) using the GitHub Apps API > > > > >>>>> > > > > > >>>>> > Most of the libraries for the GH API use a), so does > > > > Flinkbot. > > > > >>>>> The problem > > > > >>>>> > with a) is that it does not allow for fine-grained > access > > > > >>>>> control, and > > > > >>>>> > Infra does not want to give Flinkbot "write" access to > > > > >>>>> "apache/flink". > > > > >>>>> > That's why I need to rewrite parts of the bot to > support > > > b), > > > > >>>>> which allows > > > > >>>>> > to give access only a repo's metadata, but not the code > > > > itself. > > > > >>>>> > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise < > > > [hidden email] > > > > >>>>> <mailto:[hidden email]>> wrote: > > > > >>>>> > > > > > >>>>> > > It would be good to encourage participation of > > > > non-committers > > > > >>>>> in the > > > > >>>>> > review > > > > >>>>> > > process, so +1 for allowing everyone to operate the > > bot. > > > > >>>>> > > > > > > >>>>> > > Github approval will show a green checkmark for > > committer > > > > >>>> approval > > > > >>>>> > > (assuming accounts were linked via gitbox) - that > > should > > > > >> provide > > > > >>>>> > sufficient > > > > >>>>> > > orientation? > > > > >>>>> > > > > > > >>>>> > > I just noticed that flinkbot seems to act as Robert > > when > > > it > > > > >>>>> comes to > > > > >>>>> > label > > > > >>>>> > > management? I think that is confusing (besides > earning > > > > Robert > > > > >>>>> a lot of > > > > >>>>> > > extra github notification mail thanks to > participation > > on > > > > >>>>> every PR :) > > > > >>>>> > > > > > > >>>>> > > Overall flinkbot is very useful, thanks for all the > > work > > > on > > > > >>>>> it! I heard > > > > >>>>> > > positive feedback from other contributors, I think > they > > > see > > > > >> their > > > > >>>>> > > contributions are better received now. > > > > >>>>> > > > > > > >>>>> > > Thomas > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > > > > >>>>> <[hidden email] <mailto:[hidden email]>> > > > > >>>>> > wrote: > > > > >>>>> > > > > > > >>>>> > > > I will update labels only based on committer's > > > approvals > > > > >> (for > > > > >>>>> > > everything), > > > > >>>>> > > > I think that's cleaner. > > > > >>>>> > > > > > > > >>>>> > > > We can always revisit this. > > > > >>>>> > > > > > > > >>>>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > > > > >>>>> <[hidden email] <mailto:[hidden email]>> > > > > >>>>> > > > wrote: > > > > >>>>> > > > > > > > >>>>> > > > > Fore code-quality/description I agree, but > > consensus > > > > and > > > > >>>>> the final > > > > >>>>> > > > > approval should require a committer IMO. > > > > >>>>> > > > > > > > > >>>>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > > >>>>> > > > > > > > > >>>>> > > > > I did not put any restrictions on who can > > communicate > > > > with > > > > >>>>> the bot! > > > > >>>>> > > > > But since there is currently no way of overriding > > > > >>>>> somebody's approval > > > > >>>>> > > for > > > > >>>>> > > > > something, this can easily lead to such a > > situation. > > > > >>>>> > > > > > > > > >>>>> > > > > My thinking was that a committer still needs to > > > > manually > > > > >>>>> check who > > > > >>>>> > > > > approved a pull request, and I wanted to be open > > for > > > > >>>>> non-committers > > > > >>>>> > to > > > > >>>>> > > > > participate in the review process. > > > > >>>>> > > > > WIth the labels in place, this can easily send > the > > > > wrong > > > > >>>>> message. > > > > >>>>> > > > > > > > > >>>>> > > > > What should we do? > > > > >>>>> > > > > A) we restrict sending commands to the bot to > > > > committers? > > > > >>>>> > > > > B) only approvals from committers matter for > > applying > > > > >> labels? > > > > >>>>> > > > > C) we allow committers to override approvals > > > > >>>>> > > > > > > > > >>>>> > > > > I'm leaning towards B, as it encourages > > > non-committers > > > > to > > > > >>>>> > participate. > > > > >>>>> > > > > > > > > >>>>> > > > > > > > > >>>>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler > > > > >>>>> <[hidden email] <mailto:[hidden email]> > > > > >>>>> > > > > > > >>>>> > > > > wrote: > > > > >>>>> > > > > > > > > >>>>> > > > >> Just noticed that _anyone_ can approve a PR now, > > see > > > > >>>>> > > > >> https://github.com/apache/flink/pull/7801. > > > > >>>>> > > > >> > > > > >>>>> > > > >> Not sure about the solution, but as it stands it > > is > > > > >>>>> rather trivial > > > > >>>>> > to > > > > >>>>> > > > >> nuke the review process of the entire project. > > > > >>>>> > > > >> > > > > >>>>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > > > >>>>> > > > >> > Hey all, > > > > >>>>> > > > >> > > > > > >>>>> > > > >> > the flinkbot has been active for a week now, > > and I > > > > hope > > > > >>>> the > > > > >>>>> > initial > > > > >>>>> > > > >> hiccups > > > > >>>>> > > > >> > have been resolved :) > > > > >>>>> > > > >> > > > > > >>>>> > > > >> > I wanted to start this as a permanent thread > to > > > > discuss > > > > >>>>> problems > > > > >>>>> > and > > > > >>>>> > > > >> > improvements with the bot. > > > > >>>>> > > > >> > > > > > >>>>> > > > >> > *So please post here if you have questions, > > > > problems or > > > > >>>>> ideas how > > > > >>>>> > to > > > > >>>>> > > > >> > improve it!* > > > > >>>>> > > > >> > > > > > >>>>> > > > >> > > > > >>>>> > > > >> > > > > >>>>> > > > > > > > > >>>>> > > > > > > > >>>>> > > > > > > >>>>> > > > > > >>>>> > > > > >> > > > > > > > > > > > > > > |
|
|
+1
Best, Vino Bowen Li <[hidden email]> 于2019年3月23日周六 上午12:28写道： > +1, sounds good, Jark. > > On Fri, Mar 22, 2019 at 1:55 AM Fabian Hueske <[hidden email]> wrote: > > > Hi Jark, > > > > Thanks for driving the effort to integrate the Chinese website! > > > > We have the policy that new features / improvements should be documented > in > > the same PR for a long time. > > So far, this was checked by reviewers and committers but often overlooked > > or decided to add documentation in a subsequent PR. > > When we introduced the PR template, we added "Documentation" as a > dedicated > > section to remind contributors (and reviewers) about the documentation > > policy. > > I think adding an additional step in the review process to check the > > documentation would help to enforce the policy. > > > > +1 for this proposal. > > IMO, this is independent of the Chinese documentation, but it would > > certainly help to keep both versions in sync. > > > > Best, Fabian > > > > > > Am Do., 21. März 2019 um 06:10 Uhr schrieb Jark Wu <[hidden email]>: > > > > > Hi all, > > > > > > In the past discussion of Supporting Chinese Documentation for Apache > > > Flink[1], we reach a consensus to add a documentation check item to the > > > flinkbot review process. > > > > > > I propose the idea here to get some more feedbacks about this. > > > > > > The new item we want to add is: > > > > > > ``` > > > ### 6. Are English and Chinese documentation updated? > > > > > > If the pull request introduces a new feature, the feature should be > > > documented. The Flink community is maintaining both English and Chinese > > > documents. So both English and Chinese documentation should be updated. > > If > > > you are not familiar with Chinese language, please open a JIRA tagged > > with > > > the `chinese-translation` component for Chinese documentation > translation > > > and link it with current JIRA issue. If you are familiar with Chinese > > > language, you are encouraged to update both sides in one pull request. > > > ``` > > > > > > We have opened a pull request [2] to update it to the website. > > > > > > What do you think about this? > > > > > > Thanks, > > > Jark > > > > > > > > > [1] > > > > > > > > > http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Contributing-Chinese-website-and-docs-to-Apache-Flink-tt26603.html#a26890 > > > [2] https://github.com/apache/flink-web/pull/190 > > > > > > On Thu, 7 Mar 2019 at 21:48, Robert Metzger <[hidden email]> > wrote: > > > > > > > Each Jira ticket has a "last updated" field, and in a JIRA search, > you > > > can > > > > sort results by that field. > > > > > > > > So I will regularly check all Jira tickets which have been updated > > since > > > > the last time my tool checked. For all changed Jira tickets, I'll > > update > > > > the PR if the component has changed. > > > > > > > > The implementation will be a bit differently, to not run into rate > > limits > > > > with the JIRA or GitHub API. > > > > > > > > On Thu, Mar 7, 2019 at 2:40 PM Chesnay Schepler <[hidden email]> > > > > wrote: > > > > > > > > > How do you intend to keep the label up-to-date with whatever > > > > > modifications are made in JIRA? > > > > > > > > > > On 07.03.2019 13:40, Robert Metzger wrote: > > > > > > I will automatically assign the Jira component as a label to the > > PR, > > > > yes. > > > > > > You won't have to manually update the label on the PR, this will > be > > > > done > > > > > > automatically. > > > > > > > > > > > > So JIRA will stay the ground truth for setting the component > > > correctly. > > > > > > > > > > > > On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler < > > [hidden email] > > > > > > > > > wrote: > > > > > > > > > > > >> Component labels seem a bit redundant. Every JIRA with an open > PR > > > > > >> already has a "pull-request-available" tag. > > > > > >> So this information already exists. > > > > > >> > > > > > >> I assume you'll base the labels on the component tags at the > time > > > the > > > > PR > > > > > >> is opened, but this also implies that they may be set > incorrectly > > > (or > > > > > >> not at all) by the contributor. In this case we now have to > update > > > the > > > > > >> component both in JIRA and on GitHub, and I'm most certainly not > > > > looking > > > > > >> forward to that. > > > > > >> > > > > > >> On 06.03.2019 13:51, Robert Metzger wrote: > > > > > >>> This is the picture: > > > > > >>> > > > > > >> > > > > > > > > > > > > > > > https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png > > > > > >>> Speaking about feature requests, priorities and time-spend: My > > plan > > > > was > > > > > >> to > > > > > >>> now work on introducing a new label category for the > components. > > > > > >>> This should get us a lot better overview over the per-component > > > > > >>> status/health of pull requests. > > > > > >>> > > > > > >>> > > > > > >>> On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler < > > > [hidden email] > > > > > > > > > > >> wrote: > > > > > >>>> The image didn't go through. > > > > > >>>> > > > > > >>>> I would keep it as is; imo there are significantly more > > important > > > > > things > > > > > >>>> that I'd like Robert to spend time on. (literally everything > in > > > the > > > > > >>>> Feature requests section) > > > > > >>>> > > > > > >>>> If we want to better distinguish new PRs I would suggest to > > either > > > > a) > > > > > >>>> introduce a dedicated "New" label or b) not attach any label > by > > > > > default, > > > > > >>>> and only attach the description label if someone has > > > > > >>>> approved/disapproved it. > > > > > >>>> > > > > > >>>> On 06.03.2019 12:37, Robert Metzger wrote: > > > > > >>>>> Hey Kurt, > > > > > >>>>> thanks a lot for this idea. > > > > > >>>>> > > > > > >>>>> My reasoning behind using just one color is the following: I > > > wanted > > > > > to > > > > > >>>>> use one color per category of labels. > > > > > >>>>> So when we are introducing labels for components, that it'll > > look > > > > > like > > > > > >>>>> this: > > > > > >>>>> > > > > > >>>>> image.png > > > > > >>>>> > > > > > >>>>> But we could of course also go with color families per > > category. > > > So > > > > > >>>>> "review" is green colors, "component" is red colors and so > on. > > > > > >>>>> > > > > > >>>>> If nobody objects (or agrees) with me, I'll change the colors > > > soon. > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] > > > > > >>>>> <mailto:[hidden email]>> wrote: > > > > > >>>>> > > > > > >>>>> Hi Dev, > > > > > >>>>> > > > > > >>>>> I've been using the flinkbot and the label for a couple > > > days, > > > > > it > > > > > >>>>> worked > > > > > >>>>> really well! I have a minor suggestion, can we > > > > > >>>>> use different colors for different labels? We don't > need > > to > > > > > have > > > > > >>>>> different > > > > > >>>>> colors for every label, but only to distinguish whether > > > > > >>>>> someone had review the PR. > > > > > >>>>> For example, "review=description?" is the initial > default > > > > > label, > > > > > >>>>> and it may > > > > > >>>>> indicate that no reviewer has been try to review it. > > > > > >>>>> > > > > > >>>>> For "review=architecture?", "review=consensus?", > > > > > >>>>> "review=quality?", they > > > > > >>>>> indicate that at least someone has try to review it and > > > > > >>>>> approved something. It sounds like the review is in > > > progress. > > > > > >>>>> > > > > > >>>>> For "review=approved ✅", it indicates the review is > > > finished. > > > > > >>>>> > > > > > >>>>> So i think 3 colors is enough, it tell committers > whether > > > the > > > > > >>>>> review has > > > > > >>>>> not started yes, or in progress, or is finished. > > > > > >>>>> > > > > > >>>>> What do you think? > > > > > >>>>> > > > > > >>>>> Best, > > > > > >>>>> Kurt > > > > > >>>>> > > > > > >>>>> > > > > > >>>>> On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < > > > > > >> [hidden email] > > > > > >>>>> <mailto:[hidden email]>> wrote: > > > > > >>>>> > > > > > >>>>> > GitHub has two methods for authentication with the > > APIs: > > > > > >>>>> > a) using an account's oauth token > > > > > >>>>> > b) using the GitHub Apps API > > > > > >>>>> > > > > > > >>>>> > Most of the libraries for the GH API use a), so does > > > > > Flinkbot. > > > > > >>>>> The problem > > > > > >>>>> > with a) is that it does not allow for fine-grained > > access > > > > > >>>>> control, and > > > > > >>>>> > Infra does not want to give Flinkbot "write" access > to > > > > > >>>>> "apache/flink". > > > > > >>>>> > That's why I need to rewrite parts of the bot to > > support > > > > b), > > > > > >>>>> which allows > > > > > >>>>> > to give access only a repo's metadata, but not the > code > > > > > itself. > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> > On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise < > > > > [hidden email] > > > > > >>>>> <mailto:[hidden email]>> wrote: > > > > > >>>>> > > > > > > >>>>> > > It would be good to encourage participation of > > > > > non-committers > > > > > >>>>> in the > > > > > >>>>> > review > > > > > >>>>> > > process, so +1 for allowing everyone to operate the > > > bot. > > > > > >>>>> > > > > > > > >>>>> > > Github approval will show a green checkmark for > > > committer > > > > > >>>> approval > > > > > >>>>> > > (assuming accounts were linked via gitbox) - that > > > should > > > > > >> provide > > > > > >>>>> > sufficient > > > > > >>>>> > > orientation? > > > > > >>>>> > > > > > > > >>>>> > > I just noticed that flinkbot seems to act as Robert > > > when > > > > it > > > > > >>>>> comes to > > > > > >>>>> > label > > > > > >>>>> > > management? I think that is confusing (besides > > earning > > > > > Robert > > > > > >>>>> a lot of > > > > > >>>>> > > extra github notification mail thanks to > > participation > > > on > > > > > >>>>> every PR :) > > > > > >>>>> > > > > > > > >>>>> > > Overall flinkbot is very useful, thanks for all the > > > work > > > > on > > > > > >>>>> it! I heard > > > > > >>>>> > > positive feedback from other contributors, I think > > they > > > > see > > > > > >> their > > > > > >>>>> > > contributions are better received now. > > > > > >>>>> > > > > > > > >>>>> > > Thomas > > > > > >>>>> > > > > > > > >>>>> > > > > > > > >>>>> > > > > > > > >>>>> > > On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger > > > > > >>>>> <[hidden email] <mailto:[hidden email]>> > > > > > >>>>> > wrote: > > > > > >>>>> > > > > > > > >>>>> > > > I will update labels only based on committer's > > > > approvals > > > > > >> (for > > > > > >>>>> > > everything), > > > > > >>>>> > > > I think that's cleaner. > > > > > >>>>> > > > > > > > > >>>>> > > > We can always revisit this. > > > > > >>>>> > > > > > > > > >>>>> > > > On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler > > > > > >>>>> <[hidden email] <mailto:[hidden email]>> > > > > > >>>>> > > > wrote: > > > > > >>>>> > > > > > > > > >>>>> > > > > Fore code-quality/description I agree, but > > > consensus > > > > > and > > > > > >>>>> the final > > > > > >>>>> > > > > approval should require a committer IMO. > > > > > >>>>> > > > > > > > > > >>>>> > > > > On 27.02.2019 15:08, Robert Metzger wrote: > > > > > >>>>> > > > > > > > > > >>>>> > > > > I did not put any restrictions on who can > > > communicate > > > > > with > > > > > >>>>> the bot! > > > > > >>>>> > > > > But since there is currently no way of > overriding > > > > > >>>>> somebody's approval > > > > > >>>>> > > for > > > > > >>>>> > > > > something, this can easily lead to such a > > > situation. > > > > > >>>>> > > > > > > > > > >>>>> > > > > My thinking was that a committer still needs to > > > > > manually > > > > > >>>>> check who > > > > > >>>>> > > > > approved a pull request, and I wanted to be > open > > > for > > > > > >>>>> non-committers > > > > > >>>>> > to > > > > > >>>>> > > > > participate in the review process. > > > > > >>>>> > > > > WIth the labels in place, this can easily send > > the > > > > > wrong > > > > > >>>>> message. > > > > > >>>>> > > > > > > > > > >>>>> > > > > What should we do? > > > > > >>>>> > > > > A) we restrict sending commands to the bot to > > > > > committers? > > > > > >>>>> > > > > B) only approvals from committers matter for > > > applying > > > > > >> labels? > > > > > >>>>> > > > > C) we allow committers to override approvals > > > > > >>>>> > > > > > > > > > >>>>> > > > > I'm leaning towards B, as it encourages > > > > non-committers > > > > > to > > > > > >>>>> > participate. > > > > > >>>>> > > > > > > > > > >>>>> > > > > > > > > > >>>>> > > > > On Wed, Feb 27, 2019 at 2:01 PM Chesnay > Schepler > > > > > >>>>> <[hidden email] <mailto:[hidden email]> > > > > > >>>>> > > > > > > > >>>>> > > > > wrote: > > > > > >>>>> > > > > > > > > > >>>>> > > > >> Just noticed that _anyone_ can approve a PR > now, > > > see > > > > > >>>>> > > > >> https://github.com/apache/flink/pull/7801. > > > > > >>>>> > > > >> > > > > > >>>>> > > > >> Not sure about the solution, but as it stands > it > > > is > > > > > >>>>> rather trivial > > > > > >>>>> > to > > > > > >>>>> > > > >> nuke the review process of the entire project. > > > > > >>>>> > > > >> > > > > > >>>>> > > > >> On 13.02.2019 10:29, Robert Metzger wrote: > > > > > >>>>> > > > >> > Hey all, > > > > > >>>>> > > > >> > > > > > > >>>>> > > > >> > the flinkbot has been active for a week now, > > > and I > > > > > hope > > > > > >>>> the > > > > > >>>>> > initial > > > > > >>>>> > > > >> hiccups > > > > > >>>>> > > > >> > have been resolved :) > > > > > >>>>> > > > >> > > > > > > >>>>> > > > >> > I wanted to start this as a permanent thread > > to > > > > > discuss > > > > > >>>>> problems > > > > > >>>>> > and > > > > > >>>>> > > > >> > improvements with the bot. > > > > > >>>>> > > > >> > > > > > > >>>>> > > > >> > *So please post here if you have questions, > > > > > problems or > > > > > >>>>> ideas how > > > > > >>>>> > to > > > > > >>>>> > > > >> > improve it!* > > > > > >>>>> > > > >> > > > > > > >>>>> > > > >> > > > > > >>>>> > > > >> > > > > > >>>>> > > > > > > > > > >>>>> > > > > > > > > >>>>> > > > > > > > >>>>> > > > > > > >>>>> > > > > > >> > > > > > > > > > > > > > > > > > > > > |
|
|
+1
Best, Shengjk1 On 03/23/2019 12:08，vino yang<[hidden email]> wrote： +1 Best, Vino Bowen Li <[hidden email]> 于2019年3月23日周六 上午12:28写道： +1, sounds good, Jark. On Fri, Mar 22, 2019 at 1:55 AM Fabian Hueske <[hidden email]> wrote: Hi Jark, Thanks for driving the effort to integrate the Chinese website! We have the policy that new features / improvements should be documented in the same PR for a long time. So far, this was checked by reviewers and committers but often overlooked or decided to add documentation in a subsequent PR. When we introduced the PR template, we added "Documentation" as a dedicated section to remind contributors (and reviewers) about the documentation policy. I think adding an additional step in the review process to check the documentation would help to enforce the policy. +1 for this proposal. IMO, this is independent of the Chinese documentation, but it would certainly help to keep both versions in sync. Best, Fabian Am Do., 21. März 2019 um 06:10 Uhr schrieb Jark Wu <[hidden email]>: Hi all, In the past discussion of Supporting Chinese Documentation for Apache Flink[1], we reach a consensus to add a documentation check item to the flinkbot review process. I propose the idea here to get some more feedbacks about this. The new item we want to add is: ``` ### 6. Are English and Chinese documentation updated? If the pull request introduces a new feature, the feature should be documented. The Flink community is maintaining both English and Chinese documents. So both English and Chinese documentation should be updated. If you are not familiar with Chinese language, please open a JIRA tagged with the `chinese-translation` component for Chinese documentation translation and link it with current JIRA issue. If you are familiar with Chinese language, you are encouraged to update both sides in one pull request. ``` We have opened a pull request [2] to update it to the website. What do you think about this? Thanks, Jark [1] http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Contributing-Chinese-website-and-docs-to-Apache-Flink-tt26603.html#a26890 [2] https://github.com/apache/flink-web/pull/190 On Thu, 7 Mar 2019 at 21:48, Robert Metzger <[hidden email]> wrote: Each Jira ticket has a "last updated" field, and in a JIRA search, you can sort results by that field. So I will regularly check all Jira tickets which have been updated since the last time my tool checked. For all changed Jira tickets, I'll update the PR if the component has changed. The implementation will be a bit differently, to not run into rate limits with the JIRA or GitHub API. On Thu, Mar 7, 2019 at 2:40 PM Chesnay Schepler <[hidden email]> wrote: How do you intend to keep the label up-to-date with whatever modifications are made in JIRA? On 07.03.2019 13:40, Robert Metzger wrote: I will automatically assign the Jira component as a label to the PR, yes. You won't have to manually update the label on the PR, this will be done automatically. So JIRA will stay the ground truth for setting the component correctly. On Thu, Mar 7, 2019 at 10:11 AM Chesnay Schepler < [hidden email] wrote: Component labels seem a bit redundant. Every JIRA with an open PR already has a "pull-request-available" tag. So this information already exists. I assume you'll base the labels on the component tags at the time the PR is opened, but this also implies that they may be set incorrectly (or not at all) by the contributor. In this case we now have to update the component both in JIRA and on GitHub, and I'm most certainly not looking forward to that. On 06.03.2019 13:51, Robert Metzger wrote: This is the picture: https://user-images.githubusercontent.com/89049/53882383-7fda9380-4016-11e9-877d-10cdc00bdfbd.png Speaking about feature requests, priorities and time-spend: My plan was to now work on introducing a new label category for the components. This should get us a lot better overview over the per-component status/health of pull requests. On Wed, Mar 6, 2019 at 12:58 PM Chesnay Schepler < [hidden email] wrote: The image didn't go through. I would keep it as is; imo there are significantly more important things that I'd like Robert to spend time on. (literally everything in the Feature requests section) If we want to better distinguish new PRs I would suggest to either a) introduce a dedicated "New" label or b) not attach any label by default, and only attach the description label if someone has approved/disapproved it. On 06.03.2019 12:37, Robert Metzger wrote: Hey Kurt, thanks a lot for this idea. My reasoning behind using just one color is the following: I wanted to use one color per category of labels. So when we are introducing labels for components, that it'll look like this: image.png But we could of course also go with color families per category. So "review" is green colors, "component" is red colors and so on. If nobody objects (or agrees) with me, I'll change the colors soon. On Wed, Mar 6, 2019 at 7:51 AM Kurt Young <[hidden email] <mailto:[hidden email]>> wrote: Hi Dev, I've been using the flinkbot and the label for a couple days, it worked really well! I have a minor suggestion, can we use different colors for different labels? We don't need to have different colors for every label, but only to distinguish whether someone had review the PR. For example, "review=description?" is the initial default label, and it may indicate that no reviewer has been try to review it. For "review=architecture?", "review=consensus?", "review=quality?", they indicate that at least someone has try to review it and approved something. It sounds like the review is in progress. For "review=approved ✅", it indicates the review is finished. So i think 3 colors is enough, it tell committers whether the review has not started yes, or in progress, or is finished. What do you think? Best, Kurt On Mon, Mar 4, 2019 at 6:50 PM Robert Metzger < [hidden email] <mailto:[hidden email]>> wrote: GitHub has two methods for authentication with the APIs: a) using an account's oauth token b) using the GitHub Apps API Most of the libraries for the GH API use a), so does Flinkbot. The problem with a) is that it does not allow for fine-grained access control, and Infra does not want to give Flinkbot "write" access to "apache/flink". That's why I need to rewrite parts of the bot to support b), which allows to give access only a repo's metadata, but not the code itself. On Sat, Mar 2, 2019 at 12:42 AM Thomas Weise < [hidden email] <mailto:[hidden email]>> wrote: It would be good to encourage participation of non-committers in the review process, so +1 for allowing everyone to operate the bot. Github approval will show a green checkmark for committer approval (assuming accounts were linked via gitbox) - that should provide sufficient orientation? I just noticed that flinkbot seems to act as Robert when it comes to label management? I think that is confusing (besides earning Robert a lot of extra github notification mail thanks to participation on every PR :) Overall flinkbot is very useful, thanks for all the work on it! I heard positive feedback from other contributors, I think they see their contributions are better received now. Thomas On Fri, Mar 1, 2019 at 8:38 AM Robert Metzger <[hidden email] <mailto:[hidden email]>> wrote: I will update labels only based on committer's approvals (for everything), I think that's cleaner. We can always revisit this. On Wed, Feb 27, 2019 at 4:31 PM Chesnay Schepler <[hidden email] <mailto:[hidden email]>> wrote: Fore code-quality/description I agree, but consensus and the final approval should require a committer IMO. On 27.02.2019 15:08, Robert Metzger wrote: I did not put any restrictions on who can communicate with the bot! But since there is currently no way of overriding somebody's approval for something, this can easily lead to such a situation. My thinking was that a committer still needs to manually check who approved a pull request, and I wanted to be open for non-committers to participate in the review process. WIth the labels in place, this can easily send the wrong message. What should we do? A) we restrict sending commands to the bot to committers? B) only approvals from committers matter for applying labels? C) we allow committers to override approvals I'm leaning towards B, as it encourages non-committers to participate. On Wed, Feb 27, 2019 at 2:01 PM Chesnay Schepler <[hidden email] <mailto:[hidden email]> wrote: Just noticed that _anyone_ can approve a PR now, see https://github.com/apache/flink/pull/7801. Not sure about the solution, but as it stands it is rather trivial to nuke the review process of the entire project. On 13.02.2019 10:29, Robert Metzger wrote: Hey all, the flinkbot has been active for a week now, and I hope the initial hiccups have been resolved :) I wanted to start this as a permanent thread to discuss problems and improvements with the bot. *So please post here if you have questions, problems or ideas how to improve it!* |
«
Return to (DEPRECATED) Apache Flink Mailing List archive.
|
1 view|%1 views
| Free forum by Nabble | Edit this page |