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: 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 <ykt...@gmail.com> 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 <rmetz...@apache.org> 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 <t...@apache.org> 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 <rmetz...@apache.org> > > 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 <ches...@apache.org > > > > > > 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 < > ches...@apache.org > > > > > > > > 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!* > > > > >> > > > > > >> > > > > >> > > > > > > > > > > > > > > >
