Summary: | Cleanup: ResourceResponse should be a class, not a struct | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Nordman <michaeln> | ||||
Component: | WebCore Misc. | Assignee: | Darin Fisher (:fishd, Google) <fishd> | ||||
Status: | RESOLVED INVALID | ||||||
Severity: | Normal | CC: | abarth, ap, eric, fishd | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | 30637, 30670 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Michael Nordman
2009-08-04 15:42:55 PDT
Created attachment 34184 [details]
struct -> class patch
Not much here, just some house cleaning. I think I've got all of them in the webkit repository.
There are a few in chrome's repository to get as well once this lands and rolls into view for us there.
Comment on attachment 34184 [details]
struct -> class patch
commit-queue- because I need to land this in conjunction with a chromium patch.
I feel like we've changed *this particular class* back and forth like 3 times now, no? Several people landed build fixes for their ports without looking at the bigger picture, yes. This should resolve the problem for real. Re-assigning to fishd per Comment #2. It's been 2 weeks, just wanted to check if this should still be open. This hasn't been landed yet. Just checking in again, seeing this is still in the to-be-committed list after another week and a half. :) Really not trying to be a pest... (In reply to comment #8) > Just checking in again, seeing this is still in the to-be-committed list after > another week and a half. :) Really not trying to be a pest... We're holding off until the chromium port is landed in webkit's svn to avoid the dreaded two-sided patch landing stunt maneuver. I suspect that this patch will be long out of date by the time that happens. We should probably mark this r- once this no longer applies and you can post a new patch once you're ready to land. Comment on attachment 34184 [details]
struct -> class patch
Yeah:
1 out of 1 hunk FAILED -- saving rejects to file WebCore/loader/DocumentThreadableLoader.h.rej
I'm happy to r+ a patch once you're closer to ready to land. As is, seems silly to leave this in the queue.
How much trouble will it be if this is landed separately, with Chromium changes to follow? It's been a long wait. Is there any pressing need to get this simple cosmetic change landed? Is having this sit in the Q really an issue? To answer your question, it's no more trouble than the average webkit patch that breaks our build for some relatively simple reason... our canary goes red shortly after things land, and the poor soul serving as webkit gardener has to figure out why... and when rolling DEPS (switching what revision we build the mainline against) to the revision with the breaking change, be sure to commit the chrome-side changes with that DEPS roll. That is the so called two-sided patch process. Now if there's nothing else snaking thru the system to resolve, its not a big deal. But if at the same time we're working thru some other build issues (internal or webkit related which can pop up at any time)... it can be not a lot of fun. We're working on getting the chromium port setup as a first class citizen which will make the dreaded two-sided patch a thing of the past (i hope). I don't think there is any rush on this patch, no. I don't think that it makes much sense for patches which do not apply to sit r+'d in the to-be-committed list. :) I try and look through the to-be-committed list about once a week and make sure that patches from 3rd parties aren't just rotting there waiting for commit, or that bugs aren't sitting open which have already been landed, etc. Having un-actionable patches there is just a distraction. (In reply to comment #14) > I don't think there is any rush on this patch, no. > > I don't think that it makes much sense for patches which do not apply to sit > r+'d in the to-be-committed list. :) I try and look through the > to-be-committed list about once a week and make sure that patches from 3rd > parties aren't just rotting there waiting for commit, or that bugs aren't > sitting open which have already been landed, etc. Having un-actionable patches > there is just a distraction. So w're ok to have this sit around with r- on it for a bit? There are lots and lots and lots of open bugs in bugzilla with r-'d patches on them. :) No one will notice. Sad that this is still in purgatory. bug 30637 was a dupe. This bug is being slowly done by other bugs anyway, it's soon going to be uneeded. bug 30670 is the latest one. Looks like this has been taken care of in a piecemeal fashion. Marking as RESOLVED/INVALID. |