Commit after approval

[mgledi]

Why? Engineers tend to commit last time changes to PRs that were already reviewed. From my experience, the code that is commited then is less tested and often the reason for bugs. But it also makes sense to allow engineers to do smaller fixes after an approval and don't bother others to review again.

What? Provide a overview with PRs that have an commit after the last approval.

[morucci]

That's an interesting idea. However we need to be careful how we implement such metrics as it is very dependent from the code review provider. For instance Gerrit drops approvals when change's code is updated. GitHub keeps the approvals indeed, and that makes PRs update after approval dangerous. So for Gerrit such metric will be useless.

It seems the best way to expose such "dangerous" changes is via the query language e.g. :"from:now-3weeks and repo:sf-config and updated_after_approval". Other kind of advanced filters, we could think of "review_count: 0" or "comment_authors_count > 10".

To enable this capability we need to enhance change objects in Elasticsearch. Here are some related RFE: https://github.com/change-metrics/monocle/issues/527 and https://github.com/change-metrics/monocle/issues/370

[mgledi]

I agree. If this just becomes an annotation on your objects it would be best to integrate it generally.