Hi, I've seen a few people rolling out patches without leaving adequate explanations on the bugs. In my opinion, rolling out patches without explaining the reason is actively harmful to the project because:
- The author and the reviewer have no idea what caused the patch to be rolled out. - If build failure or test failure occurred on ports/platforms they don't normally work, then they have no clue what those failures were. So when you're rolling out a patch, please leave the following information on the original bug (not the one sheriff bot automatically creates): - Did builds fail? If so, on which bots, ports, platforms were they and what were errors? - Did tests fail? If so, on which bots, ports, platforms were they and what were diffs / errors? For example, "test was failing" can't be considered as an adequate description because it doesn't tell us which tests failed on which port and platform and what were diffs / errors. It must be something along the line of "test X Y Z were failing on port A platform B, and there's the diff on A-B." (with an appropriate link to build bots' result pages). Also, given the number of reviewers and distributions among different time zones, it might make sense for us to require a review to roll out patches someone else wrote (I think patch authors should be able to rollout their patches at their discretion) since any reviewer can r+ rollout patches. Best, Ryosuke Niwa Software Engineer Google Inc.
_______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
