-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Team nagboard #61
Comments
Sounds great!
|
@brson Are there any existing examples of these comments from team members? It'd help to have some actual GitHub comments to use for testing as I build this. |
cc @aturon |
I mentioned this on IRC but it's at https://raw.githubusercontent.com/rust-lang/rust-www/master/team.md
No, the |
If you need to test this stuff we can open demo issues and people can leave comments. |
Note that I had not envisioned the bot tallying "votes" -- just tracking who has given feedback and who hasn't. (I don't even think of these as votes per se, since we operate on a consensus plan typically -- more just tracking who has given their opinion and is satisfied with status quo). |
@nikomatsakis Sure thing. So are the If there's no voting or tallying needed beyond a simple total on the dashboard, I can look at implementing notifications as simply as possible. Email? |
@brson Some demo issues w/ comments would be fantastic. In my dev database I've got a bunch of issues and comments built, and it's a pretty simple set of criteria for display, but I always prefer to test on "real" data before deploying. |
@dikaiosune I created some test issues and pinged some people to fill in some comments |
Great! Hopefully with a quick look at some test data I'll be able to Adam On Wed, Jun 22, 2016 at 1:59 PM, Brian Anderson notifications@github.com
|
Just a heads up that I've been holding on this since @aturon pinged me on #rust-internals on Thursday:
|
@brson @aturon I've managed to carve out some time to work on this, so in the next few days I'll be implementing the behavior specified at https://internals.rust-lang.org/t/refining-rfcs-part-3-async-decisions/3658/33. If you're feeling antsy (I know I am), you can follow along in the rfcbot branch of this repo. I'd like to get something minimal online ASAP. (oh, I know @ubsan was also very interested about the progress of this) |
I've got most of the backend code together for the bot (although not nearly as tested as I'd like). Last piece for the actual bot is to have it comment on the issues to notify users of the current status of issues. I'm not quite sure how chatty it should be -- but I figure I can start out verbose and scale it back until only information which is useful in practice is communicated. Next up are some JSON endpoints to fetch the current state for a user or an issue/RFC, and then to render it on the front-end. |
Thanks @dikaiosune! |
Still not 100% on all the bot interactions, but it's a start: anp/rfcbottest#1 More testing to do re: reviews & feedback requests, and then I need to add followup notifications and "everyone's reviewed" notifications. |
@dikaiosune very cool! One nit: when listing the people from whom reviews are requested, could we list them with GH checkboxes? i.e., |
@nikomatsakis I don't see why not. Are you suggesting that reviewers check the box to indicate they've reviewed? Or that the bot edit the comment to indicate they've reviewed once they leave a "reviewed" comment per https://internals.rust-lang.org/t/refining-rfcs-part-3-async-decisions/3658/33 ? |
@dikaiosune This looks fantastic! 🏆 re: reviewing, I think just having people check off their box would work fine as well. One other issue: rather than the bot posting responses, is it possible for the bot to edit its initial FCP comment? For example, active concerns raised by team members could be linked under their name, and the links removed (or struck out) when the concern is resolved? To keep noise to a minimum, I think the bot should generally post roughly four kinds of comments:
I think all other activity can take the form of updates to the FCP comment, which acts as a kind of status. |
@dikaiosune ah, hmm, I was hoping people could just check the box in lieu of a comment, but I forgot that we had said everyone would leave a comment. If that's too hard though a comment is fine. Is it possible (hard?) for the bot to check-off a check-box? Ah, I see @aturon posted in the meantime -- like him, I'd like the bots "motion to do X" comment to serve as a kind of quick status board where you can see what is current state of play. |
I don't believe GitHub's API tells the client who edited a comment -- would it be OK for any person with edit access to the issue/comment/repo be able to mark any reviewer as "reviewed"? My gut reaction is that reviews should be restricted to the person being marked as having reviewed the issue. But perhaps that isn't as important as I'm thinking.
It's not too hard to register checking the checkboxes, as AFAIK it's just an edit to the comment markdown which is easily parsed. However it's still not possible to authenticate those edits beyond "any person who has edit permissions to the issue comments."
Definitely possible, it's just an edit to the existing comment. Note to self: I need to come up with a solution for when command comments are edited or deleted in a way which changes the command. |
That'd make your bot smarter than @bors! :) |
@dikaiosune I am not worried about people checking names that are not their own. |
@nikomatsakis Cool. Thinking about it now, the checkbox solution also provides a convenient escape hatch for the vacation/extended absence problem without having to build a whole interface for registering those absences. |
@dikaiosune ah that's a good point :) |
Alright, after modifications to support tracking the overall status in a single comment, here's the most recent test I did: I don't yet have a good solution to edited command comments, except to ignore them if they're already in the database. I suspect that for now that's actually a fairly low-impact thing to fix. I haven't yet tested the "one week after FCP starts, post another comment," but it's pretty straightforward code so hopefully won't be an issue. This is running locally on my machine, but if everyone thinks that the level of noise and comment format(s) are OK, I'll do some cleanup, put it live, and point it at all the Rust repos. EDIT: Note that I haven't built the web interface to this yet, just in case the data model ended up needing to change. |
@dikaiosune 💯 this pretty much made my day. Ship it! |
@dikaiosune Looks like it went through now, interesting... |
@dikaiosune Any chance the concern list can link to the comment raising the concern? I noticed that you can only have one concern per comment, but I think that's a good thing -- just something people need to be aware of. |
@aturon Definitely. There are several things people need to be aware of that have yet to be documented :). BTW, where do you think a good location for those docs would be? I could stick it in a markdown doc in the repo, but that seems maybe a bit out of the way. |
@dikaiosune Seems like a great use for The Forge. |
I shall fetch my hammer! |
OK, the barebones web view is live: As of right now, it only shows review requests that haven't been completed yet, so they disappear once the review has been checked off on the rfcbot comment. Also, @brson if you want to register those concerns on rust-lang/rfcs#1636 separately you may want to edit that original comment to only be one concern and submit the others as separate comments. Unless we want to support multiple concerns per comment? |
@aturon The concern list now links to the initiating comment, rather than pinging the author: rust-lang/rfcs#1636 (comment). I will try to find some time for docs in the next little bit, although the next 2 weeks are pretty bonkers for me. |
@aturon @brson PR in to the forge with docs for rfcbot: rust-lang/rust-forge#15 I've written a quick document here on the commands it accepts and the current constraints on what it'll do. |
@dikaiosune re notifications - I think it would be useful and important to get notification set up. Posting notifications to irc is really easy and low cost (to #rust-bots, so as not to pollute other channels), although I'm not sure exactly how valuable it is. Email would be better, but I sympathise with the pain of getting it working. For triage bot I ended up sending via gmail using nodemailer. |
@nrc is github not sending notifications from the initial status update comments? Or are you referring to other intermediate items (like so-and-so has checked a box or registered a concern)? |
@dikaiosune it probably is, but I get so many GH notifications that they get lost - I have no way to mark pings as special, afaik (I could just get notifications for pings, I guess, but that is still a lot of GH mail). |
@dikaiosune Hey there! We've been loving the bot! Quick question/request: when the bot posts comments like rust-lang/rfcs#1682 (comment), AFAICT it's "going into FCP" in its own head (in that, according to the docs, it'll post a follow-up comment one week later). One problem is, it's not actually flagging the RFC as final-comment-period, and the comment it leaves is a little unclear. After discussion with @nikomatsakis, we feel comfortable with the bot fully triggering FCP, given our experience so far. That would mean:
A couple other details:
|
Bug report: the bot doesn't seem to've caught the request here |
Nevermind the last bug report -- just had a very long turnaround time. |
Feature request: the person who moves to FCP should automatically have their checkbox ticked. I often find I forget to check my own box. |
Request: I'd like to change the text on the initial comment. Today it says:
To make this more clear, can we say:
|
Feature request: global dashboard. In particular, it'd be great for http://rusty-dash.com/fcp to list each subteam with its open FCP proposals underneath -- ideally, with a "days since proposed" counter which acts as the sort order. That will make it easier for teams to keep on top of stragglers. Conversely, the list of team members is not so important to have here. |
What I would like is a list of the FCPs for each team, along with a matrix showing who has yet to respond, so I can nag them. =) |
Bug report: It looks like |
Feature request: work with https://github.com/rust-lang-nursery/fmt-rfcs I think all that would need would be using the style team for all PRs, rather than looking for a T- label. (Which is probably useful functionality to have for other repos too). |
So exciting! I'm really glad to see the amount of use the bot is getting. There are a few more requests/bugs than I anticipated landing in this thread when I suggested to continue using it, so I'm going to break these out into separate issues under the The new issues should show up as "referencing" this one right underneath my comment. |
This is to help the teams manage their workflows.
Each team member (rust-lang/Core, compiler, Lang, Libs, tools) gets their own page, like
rusty-dash.com/nag/brson
. This page shows issues/PRs that require a response.It supports two types of 'nags':
First, issues written by a team member, with comments containing
f? @user @team etc.
, require feedback. When these comments are created they go onto the mentioned users' nag board. When subsequently the user leaves any comment, the nag is satisfied and it leaves the nag board.Second, issues written by a team member, containing both the
final-comment-period
andT-*
tag, require a vote. When these comments are created they go onto the nag board of the team members associated with theT-*
tag. That user must subsequently leave a comment containing eitherfcp r+
,fcp r-
orfcp abstain
, after which the nag is cleared.@dikaiosune This one is higher priority than the other new dashboards we've discussed because it will allow for dev process changes.
ISTM that new nags should also trigger some kind of notification, not just the passive addition to the nag board, but that may be the role of another bot.
cc @nikomatsakis @nrc
The text was updated successfully, but these errors were encountered: