1028996 - [Skia] Remove patches directory and clarify commit procedure

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[George Wright (:gw280) (needinfo me!)]

There is some confusion over the patches directory. We are no longer using it because the focus is now on ensuring that any changes we make to Skia are upstreamed.

- Remove the patches directory to avoid this confusion
- Add a README_COMMITTING file explaining that any patches made to skia must be upstreamed simultaneously.

Comment 1

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-1028996-Remove-patches-directory-in-Skia-and-cla.patch

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8444539 [details] [diff] [review]
0001-Bug-1028996-Remove-patches-directory-in-Skia-and-cla.patch

Review of attachment 8444539 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/skia/README_COMMITTING
@@ +1,1 @@
> +Any changes to the Skia tree should be upstreamed simultaneously.

I would say go a little futher here? As in, we won't accept changes which are not upstreamed. At all.

Comment 3

[George Wright (:gw280) (needinfo me!)]

I'd rather leave it more flexible for now whilst we're dealing with Skia content work. So long as the patches are submitted upstream and a conversation is started (and followed up) with them, then I think that's all we can realistically ask for for now.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to George Wright (:gw280) from comment #3)
> I'd rather leave it more flexible for now whilst we're dealing with Skia
> content work. So long as the patches are submitted upstream and a
> conversation is started (and followed up) with them, then I think that's all
> we can realistically ask for for now.

I think things could easily fall through the cracks there, and then we'll end up clobbering stuff when we take a new skia update. Maybe you could mark bugs that are pending upstreaming in Skia? That way you could at least find stuff like that.

Comment 5

[George Wright (:gw280) (needinfo me!)]

That's a very good idea. We can use the [skia-upstream] whiteboard tag to keep track and require that they are submitted upstream by the author.

Comment 6

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-1028996-Remove-patches-directory-in-Skia-and-cla.patch

How about this? I've tightened up the language in README_COMMITTING but I've left the final discretion up to Skia peers. Does that sound reasonable?

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8444644 [details] [diff] [review]
0001-Bug-1028996-Remove-patches-directory-in-Skia-and-cla.patch

Review of attachment 8444644 [details] [diff] [review]:
-----------------------------------------------------------------

I like it.

Comment 8

[George Wright (:gw280) (needinfo me!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce071553b950

Comment 9

[Jan Beich]

https://hg.mozilla.org/mozilla-central/rev/ce071553b950