pullreminders / backlog

Public backlog for http://pullpanda.com
59 stars 9 forks source link

As a user, I only want business hours counted for "review turnaround time" and "time to merge" #82

Open abinoda opened 5 years ago

abinoda commented 5 years ago

It'd 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.

It would be neat if the metrics could count business hours only. For example, if my coworker requests my review at 7pm, and I do the review at exactly 10:10am the next day, on our team we'd consider that a 10-minute turnaround (because we consider "business hours" to be 10-6). In fact, one of my coworkers has waited until the morning to submit PRs, because he said he didn't want to "mess up my stats"

davidgoate commented 5 years ago

@abinoda copied from #112 as requested:

I am actually a bit unsure myself whether this is a good idea, I can imagine many pros and cons for this - but I feel like some useful discussion could be generated in this thread which might shed some light on the perceived usefulness of the requested feature. One "side effect" we've seen since we started seriously tracking stats (and developers being very aware that stats are being tracked), perhaps due to competitive nature of leaderboards etc, occasionally people are changing behaviour with regards to doing things like assigning PR's into review after 6pm or at a weekend or on a Friday afternoon. The main reason is that if the reviewer doesn't perform the review right away, e.g. until Monday morning after a Friday PM assignment, it makes their "stats look worse". To be clear here, this isn't people being malicious to make someone else look worse by tactically assigning after they go home, it's actually the other way, our team are being more compassionate in behaviour than before and not putting things into review until back in core hours so as to not impact the other individual statistically. The obvious downside to this is that if the reviewer didn't mind doing the review, now it would not happen for a day or two over the weekend since no one is aware of the PR other than the author.

I think there could be many arguments made here about working culture, whether it's fair or right for people to create PR's for review outside of "core working hours", what pressure that can put on other people etc which could probably turn into a not very useful debate. But, assuming that this is going on and that this behaviour is voluntary and mostly out of enthusiasm/passion for the company & projects, could there be a type of filter that we could tick/untick to "deduct non working days/times" from the stats on demand so that a review which is passed at 09:30 on a Monday is counted as a 30 min review if it was assigned at 10pm on a Saturday (assuming weekend is not working time and work day set to start at 9am).

I still think we'd want to be able to easily turn the filter on and off, so that we can actually compare how the numbers are impacted. I can imagine there are some complexities to this internally, but I thought I'd throw this out there for input/thoughts on the feature request.

abinoda commented 5 years ago

@davidgoate Thanks David. Your write-up is really helpful, particularly highlighting the types of behaviors resulting from the team being data-driven. I agree with the usefulness of being able to exclude non-working hours from the stats, however this will probably sit a little lower in priority because we have another tool in the works that we're hoping provides even more meaningful data and feedback for teams.

Also, while we're on this topic – would you like to see the target success rate listed in the leaderboard for review turnaround and PR size instead of just averages?

davidgoate commented 5 years ago

@abinoda r.e. the last point:

would you like to see the target success rate listed in the leaderboard for review turnaround and PR size instead of just averages?

I hadn't really thought too much about that until now. I've been happy seeing the averages and haven't found myself wanting the ability to look at success rate per individual personally. I think seeing the average PR size in LOC for e.g. is quite useful rather than just seeing at 90% of someone's PR's did meet target it is quite nice to see their actual average number of lines for the report period.

abinoda commented 5 years ago

@davidgoate Gotcha, that's really helpful feedback. Thank you!

brad-decker commented 5 years ago

@abinoda just wanted to add in my 2 cents for the business hour discussion. One thing that has manifested naturally is a rule of law to "not assign reviewers on fridays" because it skews the reviewers time to merge metric if they cannot get to it until monday. I think a MVP approach that doesn't try to tackle the question of "business hours" is to instead allow businesses to extract non-working days from the equation. If people do work on the weekend, that's fine. Knowing that the review requests they ask for won't impact my metrics as a reviewer on those non working days is already helpful. Later we can refine the business hours rules.

abinoda commented 5 years ago

@brad-decker The idea of starting with "Working days" before dealing with "Hours" makes sense.

I think we're 👍 to move forward with this. Can't promise a specific timeline but ideally before end of Q2.

abinoda commented 5 years ago

This is now in progress

abinoda commented 5 years ago

Just logging another +1 for this by @severin

brad-decker commented 5 years ago

@abinoda do you have any insight into where this will now fall with the GitHub acquisition?

abinoda commented 5 years ago

Still prioritized, but will be tackled as part of a relaunched product we are working on

lukejones1 commented 4 years ago

Still prioritized, but will be tackled as part of a relaunched product we are working on

@abinoda Is there a date set for the relaunched product?