Bug 37748

Summary:

Make Sheriffbot more inspirational

Product:

WebKit

Reporter:

Daniel Bates <dbates>

Component:

Tools / Tests

Assignee:

Daniel Bates <dbates>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, cjerdonek, eric

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch with test cases	none
Patch	eric: review-
Patch with test cases	none

Description Daniel Bates 2010-04-16 23:04:15 PDT

Teach Sheriffbot some new quotes.

Comment 1 Daniel Bates 2010-04-19 19:53:50 PDT

Created attachment 53767 [details]
Patch with test cases

Comment 2 Adam Barth 2010-04-19 21:27:00 PDT

Comment on attachment 53767 [details]
Patch with test cases

Awesome!

Comment 3 Daniel Bates 2010-04-19 21:34:35 PDT

Comment on attachment 53767 [details]
Patch with test cases

Clearing flags on attachment: 53767

Committed r57868: <http://trac.webkit.org/changeset/57868>

Comment 4 Daniel Bates 2010-04-19 21:34:41 PDT

All reviewed patches have been landed.  Closing bug.

Comment 5 Daniel Bates 2010-04-19 22:13:13 PDT

Sheriffbot sat silently when you said hi after <http://trac.webkit.org/changeset/57868> :-(. Rolled out in <http://trac.webkit.org/changeset/57869> so that I can further test/debug the patch.

Comment 6 Daniel Bates 2010-04-20 00:54:08 PDT

Created attachment 53780 [details]
Patch

Comment 7 Eric Seidel (no email) 2010-04-20 01:06:01 PDT

Comment on attachment 53780 [details]
Patch

Please add unicode to your test case.

Seems fetch_ should just call parse_ internally, no?  Then parse could be _parse?

I'm surprised the convertEntities bit is needed.  I wonder where else we might need that?

BeatufifulSoup is a subclass of BeautifulStoneSoup, no need to import BeautifulStoneSoup just for HTML_ENTITIES.

regexp strings shoudl start with r"

Comment 8 Daniel Bates 2010-04-20 01:43:45 PDT

Created attachment 53782 [details]
Patch with test cases

Updated patch based on Eric's comments.

Comment 9 Daniel Bates 2010-04-20 01:44:49 PDT

(In reply to comment #7)
> (From update of attachment 53780 [details])
> [...]
> I'm surprised the convertEntities bit is needed.  I wonder where else we might
> need that?

I'll look into this further.

Comment 10 Eric Seidel (no email) 2010-04-20 01:46:57 PDT

Comment on attachment 53782 [details]
Patch with test cases

LGTM.

Comment 11 Daniel Bates 2010-04-20 20:55:10 PDT

Comment on attachment 53782 [details]
Patch with test cases

Clearing flags on attachment: 53782

Committed r57948: <http://trac.webkit.org/changeset/57948>

Comment 12 Daniel Bates 2010-04-20 20:55:17 PDT

All reviewed patches have been landed.  Closing bug.