pullreminders/backlog

Public backlog for http://pullpanda.com

backlog's Issues

As a user, I want real-time DM's for reviews I receive on my PRs

Ignore reviews that are just comments

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.

Redesigned metrics and leaderboards

Reports

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.

People

• Who has the highest percentage of reviews within <your target review time>
• Who is doing the most code reviews?
• Who is being waited on for code reviews the most?
• Who has the slowest turnaround time on reviews?
• Who shipped the most pull requests?

Pull requests

• Which pull requests took the longest to merge?
• Which pull requests took the longest to review?
• Which pull requests were the biggest?
• Which pull requests had the most reviews?
• Which pull requests had the most comments?

Questions

What metrics or stats are you most interested in?

Allow more than one unique notification rule

As a user, I want to be able to set a default timezone for my org during initial setup so that future team and personal reminders have better defaults

Support of mattermost

Hello. Is there a support of mattermost chat?
FYI mattermost is an opensource alternative to Slack.

As as user, I would like metrics and reminders to accumulate only when PRs have certain labels applied to them

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.

[GitHub Bug] Review is wrongly attributed to a team instead of a user

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:

As a user, I want to be able to onboard teammates so they get mentions and personal reminders

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.

As a user, I want to be able to receive personal reminders for my PRs that are done being reviewed

As a user, I would like to know how many approvals a pending PR already has

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.

As a user, I want to be able to receive other types of personal reminders and alerts

Reminders:

• Review Request Reminders
  • Include merged pull requests
• My pull requests that have been reviewed

Instant Alerts:

• When I receive a review request
• When a team I'm on receives a review request
• One of my PRs gets a review

As a user, I do not want to receive review request alerts for reviews I request from myself

Time of Day changes for reminders don't persist

Changing "Time(s)" creates a notification in the target channel saying it's changed, but the changes don't persist in this edit view.

As a user, I want to be able to include merged PRs in team and personal reminders

Some teams don't require reviews BEFORE merging but still want to do reviews after merging.

Pull Request Dashboard

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.

As a user, I want to snooze direct message reminders

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)

As a user, I want to land on a home screen in my GitHub org instead the team reminders screen

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.

As a user, I want the option to include *merged* PRs in team reminders

As a user, I want to schedule specific times for reminders instead of using time window and frequency

Background

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.

Improvements

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.

Conclusion

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].

[GitHub Issue] GitHub's UI for re-requesting reviews is not intuitive and mostly not known

Background

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.

Getting stuck on "waiting for author"

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.

GitHub UX issue

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/.

Suggestion for GitHub

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.

As a user, I want to be able to receive a personal notification when one of my PRs gets reviewed

Split team reminders into two lists: "Waiting for review" and "Waiting for author"

As a user, I would like auto-suggestions for labels vs free-form typing

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.

As a user, I want to receive my team reminders on-time (not late)

As a user, I want real-time DM's for review requests to teams I am on

Currently users can receive DMS for review requests to them individually. This card is to add DMs for review requests to teams as well.

As a user, I want label/string filters for personal reminders to work with CODEOWNERS workflow

Delete past reminders from Slack to keep chat history clean

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

As a user, I want to be able to send test reminders in order to confirm that I have the right settings

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.

[GitHub Bug] GitHub treats comments as reviews and has a poor UX for requesting additional rounds of reviews

Getting stuck on "waiting on author"

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.

Issues with GitHub's UX

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:

1. The primary UX issue (which, if addressed, would also fix the 2nd issue listed below) is that users are often not aware that review requests are cancelled when reviews or comments are submitted, or that they can and should re-request reviews. This leads to an unintentional workflow where users create review requests for their initial round of review, but not for additional reviews because they are often not aware they can or should
2. A secondary UX issue (which would be resolved by fixing the first issue) is that comments submitted by clicking "Add single comment" are treated as reviews and cancel the review request. This is confusing because the "Add single comment" button is displayed right beside another button labeled "Start a review", which suggests that "Add single comment" is NOT a review. Even teams that do know to re-request reviews after requested changes often are not aware that they also should do so after receiving comments.

Suggestions for GitHub

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.

Update Slack reminder message to indicate PRs that have been merged or closed

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

Convert Slack app to Workspace App

As a user, I would like Pull Reminder to support SLA & reporting on different groups

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.

As a user, I want friendlier onboarding before being redirected to sign in via Slack

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).

Increase threshold range

Add the following options to the threshold range... or just make it a text input:

• 30 minutes
• 45 minutes
• 2 days
• 3 days
• 7 days
• 30 days

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.

Add toggle for not including reviewed PRs in reminders

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.

As a user, I would like to receive DMs in Slack when a PR gets a merge conflict

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.

Send users Slack DMs for new review requests

Also see integrations/slack#382

As a user, I want to filter reminders based on the age of the oldest pending review request

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.

As a user, I want the option to include *merged* PRs in personal reminders

Improved "review turnaround SLAs" and reporting

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:

1. A way to set SLA(s) either globally or for particular repos
2. 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)
3. 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:

• SLAs set by repo (and off by default)
• Set global "work hours" (w option to customize per user if necessary)
• Simple global feed/radar and easy way to mark "missed" as a OOO
• Recognize and ignore "wip" time state via label or term

Use Sidekiq for GitHub data syncing

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.

See sidekiq/sidekiq#3824

Get direct messages for review requests

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.

Increase concurrency level of GitHub sync

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

Upgrade Postgres database

Delete old webhook events

https://devcenter.heroku.com/articles/updating-heroku-postgres-databases#updating-with-pg-copy
https://twitter.com/sidekiq

As a user, I would like to receive DM's when I am @mentioned in a PR comment

Weekly metrics summary email

As a user, I want real-time DM's for review requests to teams I am on

Improve leaderboards

• Add ability to filter by repo (prepulate from links in reminders)
• Create separate leaderboards for:
  • Reviews Completed
  • Pull Request Size
  • Review Turnaround Time
• Date filters:
  • Last 7 days
  • Last 30 days
  • This week
  • This month
  • Last week
  • Last month

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.

Default user timezones based on Slack

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.