Bug 25877 – bugs.webkit.org should have a patch size warning

• Status: NEW
• Component: WebKit Website

[Eric Seidel (no email)]

bugs.webkit.org should warn potential committers that patches larger than 20k (or pick some equally small number) are unlikely to be reviewed.  This should be in bold letters and reviewers should feel free to just r- patches larger than that on principle.  :)  I suspect this will keep our review queue smaller.  Maybe such a warning would help me keep my own patch size smaller...  (Since I currently have ~10 patches up for review myself.)

I'll post a patch with some suggested text when I next have a clean tree.

[Eric Seidel (no email)]

s/potential committers/potential contributers/

[Alexey Proskuryakov]

It may make sense to add a note to <http://webkit.org/coding/contributing.html>, but I'm against polluting the Bugzilla "Create a New Attachment" page - a lot of good first time contributors cannot even mark patches for review, which is a sure sign that this page is too complicated as it is.

[Mark Rowe (bdash)]

The Bugzilla UI is just plain awful for new contributors, and just barely tolerable for those that are used to it.  For new contributors a simpler, task-oriented UI that assisted them with the process (whether filing a bug, attaching a patch, marking a bug as fixed) would make life less complicated.  It'd probably make things more pleasant for frequent contributors too.  I guess this is all on a bit of a tangent from this bug report though.

[Ojan Vafai]

(In reply to comment #3)
> The Bugzilla UI is just plain awful for new contributors, and just barely
> tolerable for those that are used to it.

A command-line script (a la git-send-bugzilla or chromium's gcl scripts) is much more friendly for new contributors. Also, it allows for automating many of the steps on the current contributing page. For example, it can warn if your patch is >20K or lacks a Changelog. Once we have a linter, it can warn if your style is incorrect. 

[Chris Jerdonek]

(In reply to comment #2)
> It may make sense to add a note to
> <http://webkit.org/coding/contributing.html>

I can add such a note since I'm in the middle of making some changes to that page.  Do you have any suggestions on how the page can be made simpler?

Note that we can also add screen shots (for example pointing out the "patch" checkbox, etc. in the "Add Attachment" UI), though that change will make the instructions page longer and perhaps more intimidating.

[Chris Jerdonek]

(In reply to comment #0)
> bugs.webkit.org should warn potential committers that patches larger than 20k
> (or pick some equally small number) are unlikely to be reviewed.

We can also add this check to check-webkit-style (it would be simple), though strictly speaking it's not a matter of style.

[Adam Barth]

I'm not sure check-webkit-style is the best place for the check.  Although convenient, it's not really a matter of code style.  If we'd like to check this property using a bot, it's trivial to extend an existing bot or set up a new bot.

[Chris Jerdonek]

(In reply to comment #7)
> I'm not sure check-webkit-style is the best place for the check.

After thinking a bit more, I agree with you that check-webkit-style is not a good place to check patch length.

There is an advantage, though, to having a warning appear as part of a script that contributors already run. You can save time by catching the problem earlier in the process than a bot can.

Another option is having svn-create-patch emit a warning through stderr.  This way contributors would always receive a notification early on that their patch is large.

[Chris Jerdonek]

(In reply to comment #8)
> Another option is having svn-create-patch emit a warning through stderr.  This
> way contributors would always receive a notification early on that their patch
> is large.

FYI, the report for this option is here (a patch was just submitted):

https://bugs.webkit.org/show_bug.cgi?id=32582