On 15/09/2020 16:03, Brian Grinstead wrote:
> We’re rolling out a change to the review process to put more focus on
automated testing. [...]
As others have said, it's great that we're setting a clearer policy here
- thanks!
One thing did strike me as a little surprising, and could perhaps be
clarified:
> * testing-exception-other: Commits where none of the defined
exceptions above apply but it should still be landed. This should be
scrutinized by the reviewer before using it - consider whether an
exception is actually required or if a test could be reasonably added
before using it. This requires a comment from the reviewer explaining
why it’s appropriate to land without tests.
The point that caught my eye here was that it explicitly requires the
explanatory comment to be *from the reviewer*. I would have thought that
in many cases it would make sense for the *patch author* to provide such
an explanatory comment (which the reviewer would of course be expected
to scrutinize before granting r+). Is that scenario going to be
considered unacceptable?
Thanks,
JK
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
