[Webkit-unassigned] [Bug 51790] [WinCairo] Patch to download the WinCairo dependancies as part of build-webkit.

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Brent Fulgham <bfulgham at webkit.org>  2011-04-10 21:06:50 PST ---
(From update of attachment 77970)
View in context: https://bugs.webkit.org/attachment.cgi?id=77970&action=review

This looks like a really great change -- thanks for working on it!  I have a few comments below that I hope you will consider.  I'm sorry I didn't see this patch earlier. Please feel free to cc me on any WinCairo changes in the future.

> Tools/ChangeLog:11
> +

If we are going to do this (and I hope we do!), we should probably land this on a more 'official' location than my private .Mac account.

> Tools/Scripts/build-webkit:454
> +    }

General comment:  It seems like build-webkit and friends should have a variable for the Scripts location.  We've changed this once, and all of this hardcoded pathing seems pretty fragile.

> Tools/Scripts/update-webkit-cairo-libs:4
> +#

You should have yourself here.  I'm not sure you need to include Apple for this one-line implementation.

> Tools/Scripts/update-webkit-dependancy:1
> +#!/usr/bin/perl -w

General note:  It would be helpful if you could show this file as being 'moved' from 'Tools/Scripts/update-webkit-auxiliary-libs' to this new file.  (e.g., "svn cp Tools/Scripts/update-webkit-auxiliary-libs Tools/Scripts/update-webkit-dependancy'.  Then modify the new (copied) file to show the deletion of the unused stuff, and the small additional changes you made.  That way, we could see (easily) if any of the loading logic below is different.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.