Wasn't Chesnay's comment suggesting 3 states as well?: > 1) By default the bot shows a big red X next to every item; I'd prefer a > question mark here as this allows us to differentiate between rejected > and unaddressed points. It's also a bit nicer for contributors imo as it > does not have such a negative connotation.
On 22/02/2019 14:19, Robert Metzger wrote: > I will try to deploy the first version using labels today. > > Here are my responses to your comments: > > The emojis seem unnecessary, the approved label could be shorted to >> "Approved"; the >> review prefix isn't necessary here imo. > > My idea was that each system (review bot, component labeler, size > estimator) has its own prefix, that is managed only by the system. > The Guava project is doing this as well: > https://github.com/google/guava/pulls > also Kubernetes to some extend: > https://github.com/kubernetes/kubernetes/pulls > > I'm still in favor of keeping "review=approved". > > As another request, we may want to ignore flinkbot comments if they come >> from the person opening the PR. > > This is on the TODO list. I will address it with the next big development > iteration. For now, it is up to the merger to make sure that the approvals > were given by the right persons. > > but for the corresponding pictures, if need I can look for visual design >> classmates to help. what do you think? > > Afaik, we can not use custom emoji's on GitHub. Also, it seems that the use > of emojis is not very popular with the others here :) > > > Probably I am bit late to the party, but I just started using the Flinkbot. >> A big +1 for having 3 states for each step. (Pending, Approved, Rejected). >> Right now it is impossible to say that I checked e.g. consensus and decided >> that the feature requires further discussion. > > Welcome to the party :) > Did somebody else suggest already 3 states for each step? > Since this is a bigger implementation effort, and requires additional > thinking about the semantics (what happens if we have conflicting approvals > etc.) I would suggest to add this to the TODO list, and have a separate > discussion with a full proposal? > As part of this, I also want to revisit the "attention" action. > > The bot could check the diff and tag pull requests that only touch the >> docs as "Documentation". Many of these are easy to review and usually >> don't require a deeper understanding of Flink. > > Yes, I think we should automatically label pull requests such as: hotfixes, > documentation only, new contributors. > It's on my list. > > > For the labels, I will now go with the following: > > review=description? > > review=consensus? > > review=architecture? > > review=quality? > > review=approved ✅ > > > > On Fri, Feb 22, 2019 at 2:00 PM Chesnay Schepler <ches...@apache.org> wrote: > >> The bot could check the diff and tag pull requests that only touch the >> docs as "Documentation". Many of these are easy to review and usually >> don't require a deeper understanding of Flink. >> >> 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!* >>> >>
signature.asc
Description: OpenPGP digital signature
