Ah, I didn't get that. I thought you only wanted to show ❓ instead of ❌. Maybe the "disapprove" command is also a bit misleading. What it actually means is just "remove approval".
On Fri, Feb 22, 2019 at 2:29 PM Chesnay Schepler <ches...@apache.org> wrote: > 3 states for each step is effectively what I've been suggesting at the > very start. (initialize with question mark instead of red cross) > > 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!* >> > >> >> >
