Bug 38021

Summary: complete rietveld bugzilla integration
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: Tools / TestsAssignee: Ojan Vafai <ojan>
Status: RESOLVED WONTFIX    
Severity: Normal CC: abarth, atwilson, eric, jamesr, jparent, levin, slightlyoff, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 38162    
Bug Blocks:    
Attachments:
Description Flags
Patch-11001 none

Ojan Vafai
Reported 2010-04-22 18:27:33 PDT
We've gotten all the feedback we're going to get without moving forward. So, I think we should take the next steps. Some options: 1. Make --fancy-review the default. Pro: People who have the chrome extension installed and start doing real reviews and find the bugs. Con: Upload takes longer 2. Move the extension code into bugzilla. 3. Make upload put the rietveld issue in the ChangeLog so we can upload to the same issue instead of creating a new one each time. Ideally we'd start working on all three of these immediately. 1 is the simplest. I volunteer to do that assuming there are no objections. Once we've done 1, that will let us get a better sense of what the outstanding issues are with the upload/reitveld process. Sound OK?
Attachments
Patch-11001 (2.25 KB, patch)
2010-04-26 11:09 PDT, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2010-04-23 12:30:03 PDT
Sounds good to me.
Ojan Vafai
Comment 2 2010-04-26 11:09:44 PDT
Created attachment 54319 [details] Patch-11001
Eric Seidel (no email)
Comment 3 2010-04-26 14:02:23 PDT
I'm not sure I like that this is going to put rietveld bug review links in every bug I upload.
Ojan Vafai
Comment 4 2010-04-26 14:10:45 PDT
(In reply to comment #3) > I'm not sure I like that this is going to put rietveld bug review links in > every bug I upload. All it does is append the rietveld issue id to the patch description. It only shows a rietveld review link if you have the chrome extension installed. It no longer appends a review link to the bug comments, ever.
Ojan Vafai
Comment 5 2010-04-26 14:11:20 PDT
Note that I uploaded this patch using --fancy-review, so this bug is a good example of what will change (i.e. very little).
Eric Seidel (no email)
Comment 6 2010-04-26 14:16:09 PDT
Comment on attachment 54319 [details] Patch-11001 Woh, ok. We probably need to change the description name slightly.
Ojan Vafai
Comment 7 2010-04-26 14:52:50 PDT
Committed r58265: <http://trac.webkit.org/changeset/58265>
Ojan Vafai
Comment 8 2010-04-26 14:59:53 PDT
Filed bug 38143 and bug 38144 for items 2 and 3 above.
Adam Barth
Comment 9 2010-04-26 20:36:15 PDT
Reverted r58265 for reason: This change prevents me from uploading patches Committed r58282: <http://trac.webkit.org/changeset/58282>
WebKit Review Bot
Comment 10 2010-04-26 21:08:29 PDT
http://trac.webkit.org/changeset/58282 might have broken SnowLeopard Intel Release (Tests) The following changes are on the blame list: http://trac.webkit.org/changeset/58281 http://trac.webkit.org/changeset/58282
Ojan Vafai
Comment 11 2010-04-27 08:16:45 PDT
(In reply to comment #9) > This change prevents me from uploading patches Ugh. Sorry this broke you. What was the error you were getting? Also, how did it break sheriffbot?
Eric Seidel (no email)
Comment 12 2010-05-02 19:38:50 PDT
Attachment 54319 [details] was posted by a committer and has review+, assigning to Ojan Vafai for commit.
Ojan Vafai
Comment 13 2010-05-02 21:08:43 PDT
Comment on attachment 54319 [details] Patch-11001 This has been committed and rolled back.
Ojan Vafai
Comment 14 2010-08-19 13:49:08 PDT
This is obsolete. The rietveld work is going on in different patches.
Note You need to log in before you can comment on or make changes to this bug.