pullreminders / backlog Goto Github PK
View Code? Open in Web Editor NEWPublic backlog for http://pullpanda.com
Public backlog for http://pullpanda.com
This was suggested to me by @callmevlad and a couple others in the past.
On GitHub, if I request a review from someone, then that person comes in and leaves a single comment by clicking "Add single comment", it still marks treats it as if that user reviewed the pull request. This can create a less than ideal situation where Pull Reminders considers a PR reviewed (because this is what the GitHub API indicates) even though it is not.
To solve for this, PR authors should remember to re-request reviews after responding to comments. 🏅 If they click the gear icon they'll see that the person who left the comments is no longer an active reviewer, and should be re-added.
In addition, Pull Reminders can have some extra logic added to it to "ignore" reviews that are not explicit approvals/denials. I'm not a huge fan of this idea because it feels a bit hacky to diverge from how GitHub treats reviews and review requests.
The following reports are designed to be used in retros, 1:1s, and for general fun and motivation. All reports would be scope-able by team, individual, and repo.
<your target review time>
What metrics or stats are you most interested in?
Hello. Is there a support of mattermost chat?
FYI mattermost is an opensource alternative to Slack.
Our teams often open PRs to take advantage of our CI processes, Github's diff viewer, and for various other reasons.
We use Pull Reminders to only remind for PRs with certain labels (eg. ready-for-review
).
I'd like to extend this to the metrics gathered.
In other words, time-in-review should only accumulate when a PR has certain labels applied to it. Ideally this can account for all periods where a PR has a label applied (iow. labels can be removed/added at any time and the metrics should reflect that).
This will make our PR metrics more accurate.
If a user requests a review from another user, when a review is submitted by that user, the review is displayed as <user> approved these changes
. This makes sense.
If a user requests a review from a team, when a review is submitted from a user on that team, the review is displayed as <user> approved these changes on behalf of <team>
. This makes sense.
However, AFTER that review has been submitted (and the review request disappears), if a review is requested from only a user (and not a team), the subsequent review by that user is still displayed as on behalf of <team>
even though a review was requested for the user, not the team.
This is incorrect information and misrepresents what happened. Here's a screenshot showing what happens on the second review:
Currently, when someone installs Pull Reminders to their GitHub/Slack org, there is no way to opt-in teammates to connect their Slack accounts and begin receiving personal reminders/alerts. This hurts the experience and effectiveness of Pull Reminders since non-signed up users don't get mentions or direct messages.
There should be a way for signed up users to opt-in their teammates for them. It would be good to present this during initial setup as well as a static call to action when there are non-onboarded team members.
Context
Currently, we are getting pull reminders for PRs we created with a mention of the author (i.e:Waiting on @barco
). However, our process requires 2 approvals prior to a PR being merged. Currently, this information is missed in the reminder message that is posted on the channel.
Suggestion
Add an option to expose this field or make that a default. Perhaps we can even let the admins add the number of reviews required (if Github does not allow that to be pulled in) which would then let you say something like '1/2' on the title of the PR.
P.S: Thanks.
Edit: Formatting.
Some teams don't require reviews BEFORE merging but still want to do reviews after merging.
Requested by @jadametz:
The ability to look at a page and ask "which reviews need my attention?" The page might even show all PR's but highlight the ones that aren't ignored by label or title patterns.
From @james-pack:
I love getting reminded about things I care about, and I had Pull Reminders reminding me every hour. Until I got a review for a 4k LOC PR. I didn't start for several hours -- the author took about a week to write, so waiting a few hours won't matter -- and I couldn't finish the review in an hour. At that point, I didn't want to be reminded of the PR every hour. They became a distraction.
From @CariWest:
... possibly also allow a person to "snooze" a reminder (i.e., skip the next reminder)
This should make onboarding clearer, since its confusing to be dropped into the "Reminders" section and never have to click the nav link to know there are two subsections.
Nevermind! For some reason, I never saw the drop down that distinguishes between team reminders & my reminders! I see it now! (I feel blind, haha)
Hey Abi -- is it possible to tune how often the DM reminders come through? Looks like right now I'm getting them every 4 hours.
Previously, team and personal reminders were sent based on the time window and frequency chosen. Time window and frequency were a confusing and limited way of scheduling when reminders were to be sent. For example, if you wanted reminders at 8am and 3pm, you'd have to set your time window to begin at 8am and your frequency to 7 hrs:
This is not intuitive, and to make things worse, due to performance bottlenecks reminders were posted at irregular times significantly later than what you would anticipate from your settings.
To make scheduling reminders more intuitive, I've changed the way reminders are scheduled to allow you to specify the exact times you'd like to receive them. To go back to the previous scenario, you can now specify that you'd like to receive reminders at exactly 8am and 3pm.
In addition, I released performance improvements that should ensure that reminders are received within a couple minutes of their exact scheduled times.
Being able to set and receive reminders at specific times should help you setup team and personal reminders that are more tailored to your workflow. I hope you enjoy these improvements! Feel free to leave comments and questions here, open a new issue for feature requests, or email me at [email protected].
For teams using Pull Reminders with GitHub review requests, reminders include pull requests that are Waiting for review
and Waiting for author
(when this feature is enabled). Pull Reminders considers a PR to be Waiting for review
if it has review requests, or Waiting for author
if it has received reviews but does not have any pending review requests.
A common problem that teams contact me about is that their pull requests show as Waiting for author
even after an author has responded to submitted reviews or comments. This problem occurs when the author does not create new review requests after addressing the initial round of review — review requests are deleted once a comment or review is submitted by reviewers, so to change pull requests back to Waiting for review
;,authors must re-assign reviewers after responding to reviews and comments.
The images below highlight what this looks like. After I've submitted a review for requested changes, I show up under "Reviewers" with a red x beside my name. If I click the gear icon, I see that I am no longer a requested reviewer and can be assigned again.
I have found that many GitHub users are unaware of the behavior I've described above, and that GitHub allows you to request reviews multiples times from the same reviewer . This leads to sub-optimal workflows where users create review requests for their initial round of review, but then come up with ad-hoc ways of dealing with with requesting subsequent reviews/.
I believe that code review processes on GitHub would be better by default if the UI provided “Request another review” calls-to-action in context. This would lead to users naturally requesting multiple rounds of reviews instead of users resorting to ad-hoc workflows.
I've illustrated a couple of examples of what I mean below. For example, after a review request is deleted, it would be great to display a "Request another review" link beside the reviewer's name in the "Reviewers" list. Similarly, when a review or comment is submitted, it would be great to display a "Request another review from " call to action nearby.
From @mikejolley:
There are filtering options based on labels. I’m not sure if it had issues or not, but some of our labels had special chars and spaces so I wasn’t sure if it was having problems with those label names. Pulling in labels via GitHub API could improve usability there.
Currently users can receive DMS for review requests to them individually. This card is to add DMs for review requests to teams as well.
Requested by @KiranPanesar:
Auto-delete the group reminders after X hours. They are good for prompting us to check PRs. But leaves a lot of dust in the group. I don't want people coming to our devs chat and thinking it's mainly for reminders.
This looks possible using https://api.slack.com/methods/chat.delete
Right now it's a little difficult to update reminder settings because you can't tell right away whether they are set the way you want (at least for filtering). Create a "Send test reminder" button that lets users test notification settings on-demand.
A common issue that teams run into when using Pull Reminders with GitHub review requests is that pull requests are labeled as Waiting on author
even after an author responds to comments or requested changes from a reviewer.
This happens because GitHub nullifies review requests when the reviewer submits a comment or a review. Therefore, to change pull requests back to "waiting on reviewer", authors needs to re-request reviews after responding to comments or reviews.
The images below demonstrate how GitHub works. After I submit a review for requested changes, I show up under "Reviewers" with a red x. But if I click the gear icon, I see that I am no longer a requested reviewer and must be reassigned.
This is a common point of confusion caused by GitHub's UX (keep reading for more details). I have been in contact with them about it and have suggested potential solutions.
There are two problems with GitHub's UX that lead to review requests not getting re-requested even when that this the desired intent of the user:
Users are generally unaware that review requests are nullified or that they can be re-requested. But even for those that are (like me), it is difficult to use because the "Reviewers" lists reviewers with pending requests as well as those without.
As illustrated below, I believe these issues could be addressed by adding clear calls to action to "Request another review" and by making it easy to do.
I manage a very large team with over 100 repos and we have designated Tuesdays as "Reviews Day" when we go through and follow up on PRs that got swept under the rug that week.
When the bot posts a message with a large number of PRs, it would be nice if the message could be updated as PRs are closed so that we can easily see which PRs still need attention and which have already been delt with
would like to be able to config the team members into different groups like developers, QA & PO. And for each group, could set different SLA and report based on that.
Right now, after a GitHub org is connected to a Slack team, all subsequent users who visit that org in PullReminders.com will be redirected to Slack to sign in before being able to view the account.
This is a bit abrupt and lacking in context – it'd be friendlier to display a screen telling them something like Welcome! [github org] is connected to the [slack team] Slack workspace. To get started, please authorize Pull Reminders to access your Slack acocunt in [slack team]
This would also prevent the problem where people accidentally sign into the wrong Slack workspace (https://github.com/pullreminders/web/pull/36).
Add the following options to the threshold range... or just make it a text input:
From @majormoses:
Now that there are multiple rules it opens a lot of doors but it is limited by the max threshold. The use case is that normally we want to only be pinged on active pull requests every 24 hours so we use labels to filter out when waiting on a user. I'd like to setup a second reminder for further out (say over a month) to remind us about all older pull requests even if they are not active so that we can either ping the user or close it out.
From @stympy:
The only thing I would add would be additional reminder periods... 1-24 hours is fine, but I'd also like 2 days, 3 days, and 7 days. Sometimes PRs just aren't that urgent.
From @traviskroberts:
Is it possible to decrease the reminder time to lower than an hour? We have a guideline at our company to try and approve a PR within an hour. It would be great to set a reminder for 45 minutes.
From @msilber
A toggle would be super, because we often wait for a convenient time to merge these changes.
From @adamcowley:
It would be nice if reminders could happen on: A) Open Pull Requests; or B) Open Pull Requests that are pending Review. We are trying PullReminders because people forget to Review other people's code. Once the code is approved or rejected, we don't need a reminder any more. We don't actually have a problem remembering to merge.
There is a field in the pull request object called mergeable
that allows to know if a PR can be merged: https://developer.github.com/v3/git/#checking-mergeability-of-pull-requests
It would be very valuable as a user to receive a notification when a pull request goes from mergeable: true
to mergeable: false
.
Also see integrations/slack#382
The goal of this feature is to allow you to limit reminders to when a PR has been waiting on a reviewer for a certain amount of time. For example, you could only have reminders only include pull requests that have been waiting for review for more than 2 hours.
To accomplish this, we would add a new sub-option under reminder settings called something like "Limit reminders based on time waiting for review" which would pop down an input field for the user to enter the time (e.g. 2 hrs
).
Nice-to-have: add this option to direct message reminders as well
Update: scroll down and see my latest comment #24 (comment). I'm questioning whether adding a filter is really necessary.
From @matthew-cochran:
One of the things I’m looking for is the ability to track how well me meet our internal SLAs (time to address pull request) over time by developer and as a team. For this feature, I’m interested in a few things:
- A way to set SLA(s) either globally or for particular repos
- A dashboard to show who is meeting their SLAs across all the repos and who has review(s) due that have slipped (a little public shaming goes a long way, we’ll be displaying this on the public monitors over the dev department)
- General reporting to see SLA trends over time by employee – it is a goal for each of our individual contributors to meet the SLAs and it would help me manage to have a quick view to show how they are doing (usually on the 3 month time-frame, but it would be nice to be able to slice-and-dice this)
More...
Our internal service level agreement ensures pull requests don’t get stale. Our SLA is to respond to each other’s requests within a day (but I could see other people needing the ability to set both granularity and amount e.g. 5 business hours, or 2 business days, etc). The trick is not measuring the exact time (e.g. to the minute/hour), because then devs will work towards this metric which will cause them to waste lots of time context switching. It is important to me to ensure the team has blocks of time in which they can focus on coding and are not chasing squirrels. Essentially, I want them to block time at the beginning or end of the day to do the review work (whatever works for them).
As long as the SLA threshold (e.g. one business day) is met, I’m happy, so it is more of a binary thing more than a measure of time. That being said, if we do miss the threshold, I’m interested in how much over we went at a more granular unit of measure (minutes to hours).>
There might also be exceptions to the SLA if a pull request is particularly large, so I’d like the ability to pull outliers out of the overall metrics, if possible, in order to get a general sense of normal trends.
From @CariWest:
I like that the metrics is about setting a target & then identifying who met the target time for code reviewing — that seems like a more relevant statistic than how fast a code review was, which encourages speed a little high.
Maybe a good leaderboard would be "Sally has met their target review time X times in a row!" And get a streak going for how often the code review is submitted under the target time. To do this, though, you'd want to consider a couple things, I think:
- Only business hours should be counted
- What happens when someone works in a different timezone? This has happened to me a couple times, where someone's tagged me in a code review during PST business hours; unfortunately, I work on the East Coast, so I get a reminder after my working hours. I think you might address this to some extent, so it would just be making sure it's still addressed.
From @lishenzhi:
I would like Pull Reminder to support SLA & reporting on different groups... I would like to be able to config the team members into different groups like developers, QA & PO. And for each group, could set different SLA and report based on that.
From @traviskroberts:
I believe that is the crux of it yes. Our use case would be as follows: once a PR review request is created, the user is DMed to notify them of the request. If they haven’t addressed the PR (either commented or approve/deny) within 45 minutes of the initial request, they would get sent a reminder.
^ This makes me wonder if there could be an option for basing reminders around SLA's...? e.g, "remind me 1 hr before SLA is missed"
From @xinzweb:
One more feature request would be good to know the target % chart, e.g. if my target success rate of
review wait time
is 80%, then I'd like to know what's the time we are looking at. This will help me figure out a promising signal to the team that we are actually improving the overall speed of following up the review requests. The overall goal would be only highlight (reward) people who are good at reviews, and let people join them more. Also, let's the team see as people joining that wining habit, the overall health of the project got improved.
From @@philboyd:
It'd also be nice to able to select days of the week that don't affect our target. e.g. I don't expect our developers to go through a PR on the weekend, but I also don't want someone to wait until Monday to submit if they're ready Friday at EOD.
Notes by @abinoda:
GitHub data is currently synced using a scheduled task. This makes it difficult to sync accounts frequently and monitor the size of the queue.
We should switch the scheduled job to queue Sidekiq jobs with the Heroku Redis Add-on. Sidekiq supports multi-threading and having a visible queue will provide better visibility into how backed up things are. The main issue will be Heroku Dyno memory limits.
As a user, I want to be able to get direct message reminders for just my received review requests. Support customizable frequency, time of day, and weekdays.
https://www.speedshop.co/2017/10/12/appserver.html
Increase concurrency level to 10x and schedule to run every 30 minutes instead of 60 minutes
Dependent on #13
Feedback from @xinzweb:
I did a survey last week to get people's votes on what's the leader board they like to see. Major of them is looking for # of reviews/reviewer. That will help acknowledge people who review the PR and keep the iteration rolling forward.
Most Slack users already have a timezone configured, so we should default user timezones to whatever they already have set on Slack. Get timezone from https://api.slack.com/methods/users.info when creating UserNotificationRule
.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.