Summary: | WKR (new-commit-bot) is too slow | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||
Component: | Tools / Tests | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, dpranke, eric, kling, koivisto, morrita, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2013-02-18 01:15:01 PST
Created attachment 188816 [details]
Use svn log -r without updating checkout
Created attachment 188817 [details]
Merged ToT
I'm using the new implementation now. It's MUCH faster :D Created attachment 188834 [details]
Also use utf-8 encoding
Comment on attachment 188834 [details] Also use utf-8 encoding View in context: https://bugs.webkit.org/attachment.cgi?id=188834&action=review > Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:79 > + for revision in range(self._last_svn_revision + 1, self._last_svn_revision + 10): 10? > Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:85 > + if re.match(r'^\-+$', commit_log): > + continue What is this looking for? > Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:89 > + revision += 1 Doesn't the for-in-range handle incrementing the revision? Does it update it's working copy periodically? Comment on attachment 188834 [details] Also use utf-8 encoding View in context: https://bugs.webkit.org/attachment.cgi?id=188834&action=review >> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:79 >> + for revision in range(self._last_svn_revision + 1, self._last_svn_revision + 10): > > 10? This is so that when there are many commits at once or something goes wrong, we don't end up spamming IRC with hundreds of messages. >> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:85 >> + continue > > What is this looking for? It's looking for empty entries that's filled with -------... You get this when someone commits into branches. >> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:89 >> + revision += 1 > > Doesn't the for-in-range handle incrementing the revision? Oops, yeah, we can get rid of this now. (In reply to comment #6) > Does it update it's working copy periodically? No. My plan is to have restart command do it. Comment on attachment 188834 [details] Also use utf-8 encoding View in context: https://bugs.webkit.org/attachment.cgi?id=188834&action=review >>> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:79 >>> + for revision in range(self._last_svn_revision + 1, self._last_svn_revision + 10): >> >> 10? > > This is so that when there are many commits at once or something goes wrong, we don't end up spamming IRC with hundreds of messages. OK. Can you use a named constant and explain the "why" with a comment? >>> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:85 >>> + continue >> >> What is this looking for? > > It's looking for empty entries that's filled with -------... > You get this when someone commits into branches. Can you add a helper function with a descriptive name so that folks reading this code know what its supposed to do? (In reply to comment #9) > (From update of attachment 188834 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=188834&action=review > > >>> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:79 > >>> + for revision in range(self._last_svn_revision + 1, self._last_svn_revision + 10): > >> > >> 10? > > > > This is so that when there are many commits at once or something goes wrong, we don't end up spamming IRC with hundreds of messages. > > OK. Can you use a named constant and explain the "why" with a comment? I'll add a named variable like _maximum_number_of_revisions_to_avoid_spamming_irc to avoid adding a comment. > >>> Tools/Scripts/webkitpy/tool/commands/newcommitbot.py:85 > >>> + continue > >> > >> What is this looking for? > > > > It's looking for empty entries that's filled with -------... > > You get this when someone commits into branches. > > Can you add a helper function with a descriptive name so that folks reading this code know what its supposed to do? Sure. _is_empty_log? Sure. Those names sound fine. Created attachment 197004 [details]
Fixed per comments
Comment on attachment 197004 [details]
Fixed per comments
r=me
Committed r147984: <http://trac.webkit.org/changeset/147984> |