devballteam / promonitor

Monitor status of open Pull Requests
https://devballteam.github.io/promonitor
GNU General Public License v3.0
2 stars 0 forks source link

Add remainder about review time #17

Open ghost opened 5 years ago

ghost commented 5 years ago

In our team we have defined hours for reviewing pull requests. It would be nice to have some kind of notification that now everybody should look at PRs.

Hours or review could be defined in config.

lukasz-gawron commented 5 years ago

In our team we have a rule that reviewers should take care of CR at 1 hour max from time it was created. This feature may be useful for us as well, to be notified that 1 hour passed apart from constant hour for notification.

MateuszOd commented 5 years ago

the same, like changing color of a commit after defined time (e.g. in config).

ghost commented 5 years ago

Good idea @MateuszOd. Each PR could change color after (defined in config) some period of time.

That could fulfil you @lukasz-gawron needs. But should it be for example 1h after PR was created, after last commit or last activity (like comment)?

ghost commented 5 years ago

I have begin to work on this feature #19 It's what I have described in first comment.

I'm also thinking about what You (@lukasz-gawron and @MateuszOd) have been talking about. It sounds to me like "ageing" feature. So each PR could visually display how old it is. Currently it shows time of creation. But It could give PR different color or something after defined period of time. I will work on this in separate pull request.

ghost commented 5 years ago

Thats all for "reminder feature". Merged and can be used. More informations about how enable and configure it in README

Later I will create another PR for "ageing" feature.

lukasz-gawron commented 5 years ago

I have begin to work on this feature #19 It's what I have described in first comment.

I'm also thinking about what You (@lukasz-gawron and @MateuszOd) have been talking about. It sounds to me like "ageing" feature. So each PR could visually display how old it is. Currently it shows time of creation. But It could give PR different color or something after defined period of time. I will work on this in separate pull request.

In our team code review looks like that:

Fulfilling working hours requirement would be difficult to implement so we may use simpler rule e.g. use 1h from opening but counted from period in range 8-17 e.g. PR opened at 16:30 will mark to yellow at 8:30 next day)

I think it may work like that:

@irekg @MateuszOd What are your your thoughts about that?

ghost commented 5 years ago

@lukasz-gawron I think that it's very complicated feature with a lot of conditions. I will think about it. I'd rather implement simpler more generic feature than something complicated for specific use case.

We already have timer that displays time since PR was open. The simpler addition to that could be some kind of colors for this timer for higher values. Like orange when time is displayed in hours and red if in days.

Condition with opening PR during working hours could add a lot of complexity. I would like to keep code as short and simple as possible. I'm also considering simplifying implementation of PR reminder that was added recently.