[webkit-reviews] review denied: [Bug 32542] [website] Add screenshots and clearer instructions on how to submit a patch for review : [Attachment 44857] Proposed patch 2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 16 10:29:52 PST 2009


Darin Adler <darin at apple.com> has denied Chris Jerdonek
<chris.jerdonek at gmail.com>'s request for review:
Bug 32542: [website] Add screenshots and clearer instructions on how to submit
a patch for review
https://bugs.webkit.org/show_bug.cgi?id=32542

Attachment 44857: Proposed patch 2
https://bugs.webkit.org/attachment.cgi?id=44857&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Thanks for taking this on.

> +<p>WebKit uses <a href="https://bugs.webkit.org/">Bugzilla</a>

I personally don't think that using the name Bugzilla so much is helpful. I
know that's the source of the bug database, but I don't think it's the best
name to refer to the WebKit bug database -- there are lots of Bugzilla
installations. I would call it the WebKit bug database, not Bugzilla.

I feel the same way about our bug tool being named "bugzilla-tool".

> +Every contribution must have an associated bug report in Bugzilla.
> +This is true even if the contribution is not a bug fix.

I think this is wrong. Every contribution must have review. And bugs.webkit.org
is the one of the best ways to get review. But there is no rule against having
contributions go in without ever involving bugs.webkit.org as long as it is
reviewed.

Using bugs.webkit.org is best practice, but not required.

> +<p><img src="images/bugzilla_add_attachment.png"
> +alt='Bugzilla "Add an attachment" link' /></p>

I believe these "/>" at the end of the "<img>" tags aren't needed or helpful.

> +<p>Checking the patch checkbox and marking your patch <tt>review:?</tt>
> +are necessary for getting your patch reviewed by a WebKit reviewer.
> +Setting the review flag notifies WebKit reviewers by sending them an
> +automatic e-mail through the
> +<a href="http://lists.webkit.org/mailman/listinfo/webkit-reviews">
> +webkit-reviews</a> mailing list.
> +</p>

This implies that all reviewers subscript to the webkit-reviews mailing list,
but that is not true. Different reviewers have different ways of discovering
patches that might be reviewed.

review- for now so you can consider some of my feedback


More information about the webkit-reviews mailing list