Add team owners into OWNERS files for layout tests

Add team owners into OWNERS files for layout tests

R=ojan@chromium.org
BUG=679390
Review-Url: https://codereview.chromium.org/2751113002
Cr-Commit-Position: refs/heads/master@{#460372}
Committed: https://chromium.googlesource.com/chromium/src/+/fe045f9d49ebba8328b0fd7abca555bcd6c1762f

[Sergiy Byelozyorov, 2017-03-15 15:22:33]

The CQ bit was checked by sergiyb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 15:22:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sergiy Byelozyorov, 2017-03-15 15:24:07]

This includes all entries in the spreadsheet except:
 - external/wpt directory as the discussion is still ongoing on how to specify
owner teams in this directory
 - internal mailing lists, for which we do not know public mailing lists yet or
none exists

Once we have resolved the issues above, I'll upload another CL with more changes
or upload another patchset to this CL if this is not landed yet.

[commit-bot: I haz the power, 2017-03-15 16:29:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 16:29:52]

Dry run: This issue passed the CQ dry run.

[Sergiy Byelozyorov, 2017-03-16 13:57:52]

The CQ bit was checked by sergiyb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 13:58:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 15:20:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 15:20:17]

Dry run: This issue passed the CQ dry run.

[Sergiy Byelozyorov, 2017-03-27 14:48:29]

The CQ bit was checked by sergiyb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 14:48:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sergiy Byelozyorov, 2017-03-27 14:52:08]

I've asked the teams and got the mapping to public mailing lists, so this CL is
now only missing the external/wpt directory. We've agreed on a solution for this
dir and the work needed to be done is tracked in http://crbug.com/702283.

[commit-bot: I haz the power, 2017-03-27 16:03:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 16:03:54]

Dry run: This issue passed the CQ dry run.

[Sergiy Byelozyorov, 2017-03-28 14:06:33]

The CQ bit was checked by sergiyb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 14:07:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sergiy Byelozyorov, 2017-03-28 14:12:19]

Added owner teams for external/wpt and added http://crrev.com/2779053002 as a
dependency, since we should only land OWNERS to external/wpt after we start
ignoring any changes to them them on import and export.

Now this CL includes all of the information from the spreadsheet except the
directories that do not exist anymore. For such directories we need to manually
review CL that removed to find out where tests were removed completed or moved
to a different directory. Such review can be done after this CL lands. The list
of directories that were removed is here:
https://paste.googleplex.com/5142809896026112.

Ojan, PTAL and let me know if these OWNER teams look good to you.

[Sergiy Byelozyorov, 2017-03-28 14:15:39]

On 2017/03/28 14:12:19, Sergiy Byelozyorov wrote:
> Now this CL includes all of the information from the spreadsheet except the
> directories that do not exist anymore. For such directories we need to
manually
> review CL that removed to find out where tests were removed completed or moved
> to a different directory. Such review can be done after this CL lands. The
list
> of directories that were removed is here:
> https://paste.googleplex.com/5142809896026112.

I clearly can't type. Here is correct version of this paragraph:

Now this CL includes all of the information from the spreadsheet except the
directories that do not exist anymore. For such directories we need to manually
review CLs that removed them to find out whether tests were removed completely
or moved to a different directory. Such review can be done after this CL lands.
The list of directories that were removed is here:
https://paste.googleplex.com/5142809896026112.

[commit-bot: I haz the power, 2017-03-28 15:41:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 15:41:41]

Dry run: This issue passed the CQ dry run.

[cbiesinger, 2017-03-28 17:15:41]

As a sidenote, the csswg-tests repository has just been merged into wpt, so on
the next import a css/ directory with a lot of subdirectories will appear. But
that can be dealt with later.

[ojan, 2017-03-28 18:30:57]

ojan@chromium.org changed reviewers:
+ sshruthi@chromium.org

[ojan, 2017-03-28 18:31:00]

I don't see the bug components from the spreadsheet here. Did you intend to do
that in a followup?

FWIW, there's a few rows in the spreadsheet that incorrectly have a team mailing
list listed as the bug component.

In either case, this LGTM. The devtools issue and bug components can be
addressed in followup patches.

https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/inspector-enabled/OWNERS (right):

https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-enabled/OWNERS:1: # TEAM:
google-chrome-developer-tools@googlegroups.com
This doesn't look right to me. As best I can tell, this is a mailing list for
web developers to reach the devtools team. Not the list the devtools team does
to do their work.

https://groups.google.com/forum/#!forum/google-chrome-developer-tools

Is there context I'm missing or did the devtools team misunderstand the purpose
of putting email lists here?

[blink-reviews, 2017-03-28 19:14:11]

It wasn't clear what the components would be for these directories.
Currently, we only have one LayoutTests component, so we need to answer the
question of where/what to create new sub components Blink>A>LayoutTests, or
Blink>LayoutTests>A, for instance. So, landing this change with just the
team mailing lists sounds fine to me as well, with bug components coming in
followup patches, when we reach a conclusion on that.

On Tue, Mar 28, 2017 at 11:30 AM, <ojan@chromium.org> wrote:

> I don't see the bug components from the spreadsheet here. Did you intend
> to do
> that in a followup?
>
> FWIW, there's a few rows in the spreadsheet that incorrectly have a team
> mailing
> list listed as the bug component.
>
> In either case, this LGTM. The devtools issue and bug components can be
> addressed in followup patches.
>
>
> https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/
> LayoutTests/inspector-enabled/OWNERS
> File third_party/WebKit/LayoutTests/inspector-enabled/OWNERS (right):
>
> https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/
> LayoutTests/inspector-enabled/OWNERS#newcode1
> third_party/WebKit/LayoutTests/inspector-enabled/OWNERS:1: # TEAM:
> google-chrome-developer-tools@googlegroups.com
> This doesn't look right to me. As best I can tell, this is a mailing
> list for web developers to reach the devtools team. Not the list the
> devtools team does to do their work.
>
> https://groups.google.com/forum/#!forum/google-chrome-developer-tools
>
> Is there context I'm missing or did the devtools team misunderstand the
> purpose of putting email lists here?
>
> https://codereview.chromium.org/2751113002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2017-03-28 19:14:11]

It wasn't clear what the components would be for these directories.
Currently, we only have one LayoutTests component, so we need to answer the
question of where/what to create new sub components Blink>A>LayoutTests, or
Blink>LayoutTests>A, for instance. So, landing this change with just the
team mailing lists sounds fine to me as well, with bug components coming in
followup patches, when we reach a conclusion on that.

On Tue, Mar 28, 2017 at 11:30 AM, <ojan@chromium.org> wrote:

> I don't see the bug components from the spreadsheet here. Did you intend
> to do
> that in a followup?
>
> FWIW, there's a few rows in the spreadsheet that incorrectly have a team
> mailing
> list listed as the bug component.
>
> In either case, this LGTM. The devtools issue and bug components can be
> addressed in followup patches.
>
>
> https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/
> LayoutTests/inspector-enabled/OWNERS
> File third_party/WebKit/LayoutTests/inspector-enabled/OWNERS (right):
>
> https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/
> LayoutTests/inspector-enabled/OWNERS#newcode1
> third_party/WebKit/LayoutTests/inspector-enabled/OWNERS:1: # TEAM:
> google-chrome-developer-tools@googlegroups.com
> This doesn't look right to me. As best I can tell, this is a mailing
> list for web developers to reach the devtools team. Not the list the
> devtools team does to do their work.
>
> https://groups.google.com/forum/#!forum/google-chrome-developer-tools
>
> Is there context I'm missing or did the devtools team misunderstand the
> purpose of putting email lists here?
>
> https://codereview.chromium.org/2751113002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Sergiy Byelozyorov, 2017-03-28 19:32:19]

As Shruthi had mentioned, we don't have component information for layout test
directories in the spreadsheet, therefore we have decided to ask using team
names for now and add components later. Shruthi will lead the effort collecting
this information from teams.

Should I have know in advance that we'll be using components, I would have added
a column for this initially. But instead the column was added later and very few
teams filled it out.

https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/inspector-enabled/OWNERS (right):

https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector-enabled/OWNERS:1: # TEAM:
google-chrome-developer-tools@googlegroups.com
On 2017/03/28 18:31:00, ojan wrote:
> This doesn't look right to me. As best I can tell, this is a mailing list for
> web developers to reach the devtools team. Not the list the devtools team does
> to do their work.
> 
> https://groups.google.com/forum/#!forum/google-chrome-developer-tools
> 
> Is there context I'm missing or did the devtools team misunderstand the
purpose
> of putting email lists here?

I don't think we have made it clear enough to the teams what these emails were
for when we created the spreadsheet. In fact at the time we didn't know for sure
ourselves. I'll reach out to devtools team and ask which list we can use
instead.

[Sergiy Byelozyorov, 2017-03-28 19:32:34]

The CQ bit was checked by sergiyb@chromium.org

[commit-bot: I haz the power, 2017-03-28 19:33:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 19:33:14]

This CL has an open dependency (Issue 2779053002 Patch 20001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Sergiy Byelozyorov, 2017-03-28 19:35:20]

The CQ bit was unchecked by sergiyb@chromium.org

[ojan, 2017-03-29 01:06:27]

I was picturing it would just be the components already in the spreadsheet,
e.g. animations has listed Blink>Animations. We can use the ones already in
the spreadsheet and then reach out to teams for the rest since we know
mailing lists for them now. :)

I don't think we need new components at all here though. LayoutTest
failures don't need a special component. They can go in the team's regular
component. I expect most teams will want to do it that way. But if they
want a new component, they can do that as a one off. No need for
consistency here as long as the team maintains responsibility for staying
on top of their components.

In that regard, I think we should definitely *not* have Blink>LayoutTests
as a component. We used to have things like that and it was very confusing
what component to give things.

On Tue, Mar 28, 2017 at 12:14 PM Shruthi Sreekanta <sshruthi@google.com>
wrote:

> It wasn't clear what the components would be for these directories.
> Currently, we only have one LayoutTests component, so we need to answer the
> question of where/what to create new sub components Blink>A>LayoutTests, or
> Blink>LayoutTests>A, for instance. So, landing this change with just the
> team mailing lists sounds fine to me as well, with bug components coming in
> followup patches, when we reach a conclusion on that.
>
> On Tue, Mar 28, 2017 at 11:30 AM, <ojan@chromium.org> wrote:
>
> I don't see the bug components from the spreadsheet here. Did you intend
> to do
> that in a followup?
>
> FWIW, there's a few rows in the spreadsheet that incorrectly have a team
> mailing
> list listed as the bug component.
>
> In either case, this LGTM. The devtools issue and bug components can be
> addressed in followup patches.
>
>
>
>
https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/inspector-enabled/OWNERS (right):
>
>
>
https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/inspector-enabled/OWNERS:1: # TEAM:
> google-chrome-developer-tools@googlegroups.com
> This doesn't look right to me. As best I can tell, this is a mailing
> list for web developers to reach the devtools team. Not the list the
> devtools team does to do their work.
>
> https://groups.google.com/forum/#!forum/google-chrome-developer-tools
>
> Is there context I'm missing or did the devtools team misunderstand the
> purpose of putting email lists here?
>
> https://codereview.chromium.org/2751113002/
>
>
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[ojan, 2017-03-29 01:06:27]

I was picturing it would just be the components already in the spreadsheet,
e.g. animations has listed Blink>Animations. We can use the ones already in
the spreadsheet and then reach out to teams for the rest since we know
mailing lists for them now. :)

I don't think we need new components at all here though. LayoutTest
failures don't need a special component. They can go in the team's regular
component. I expect most teams will want to do it that way. But if they
want a new component, they can do that as a one off. No need for
consistency here as long as the team maintains responsibility for staying
on top of their components.

In that regard, I think we should definitely *not* have Blink>LayoutTests
as a component. We used to have things like that and it was very confusing
what component to give things.

On Tue, Mar 28, 2017 at 12:14 PM Shruthi Sreekanta <sshruthi@google.com>
wrote:

> It wasn't clear what the components would be for these directories.
> Currently, we only have one LayoutTests component, so we need to answer the
> question of where/what to create new sub components Blink>A>LayoutTests, or
> Blink>LayoutTests>A, for instance. So, landing this change with just the
> team mailing lists sounds fine to me as well, with bug components coming in
> followup patches, when we reach a conclusion on that.
>
> On Tue, Mar 28, 2017 at 11:30 AM, <ojan@chromium.org> wrote:
>
> I don't see the bug components from the spreadsheet here. Did you intend
> to do
> that in a followup?
>
> FWIW, there's a few rows in the spreadsheet that incorrectly have a team
> mailing
> list listed as the bug component.
>
> In either case, this LGTM. The devtools issue and bug components can be
> addressed in followup patches.
>
>
>
>
https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/inspector-enabled/OWNERS (right):
>
>
>
https://codereview.chromium.org/2751113002/diff/50001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/inspector-enabled/OWNERS:1: # TEAM:
> google-chrome-developer-tools@googlegroups.com
> This doesn't look right to me. As best I can tell, this is a mailing
> list for web developers to reach the devtools team. Not the list the
> devtools team does to do their work.
>
> https://groups.google.com/forum/#!forum/google-chrome-developer-tools
>
> Is there context I'm missing or did the devtools team misunderstand the
> purpose of putting email lists here?
>
> https://codereview.chromium.org/2751113002/
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Sergiy Byelozyorov, 2017-03-29 13:35:34]

The CQ bit was checked by sergiyb@chromium.org

[commit-bot: I haz the power, 2017-03-29 13:35:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 13:48:38]

CQ is committing da patch.

Bot data: {"patchset_id": 50001, "attempt_start_ts": 1490794534178360,
"parent_rev": "65d6e34d665cd4ad6c23066d1500e0fa8eae2070", "commit_rev":
"fe045f9d49ebba8328b0fd7abca555bcd6c1762f"}

[commit-bot: I haz the power, 2017-03-29 13:49:28]

Description was changed from

==========
Add team owners into OWNERS files for layout tests

R=ojan@chromium.org
BUG=679390
==========

to

==========
Add team owners into OWNERS files for layout tests

R=ojan@chromium.org
BUG=679390

Review-Url: https://codereview.chromium.org/2751113002
Cr-Commit-Position: refs/heads/master@{#460372}
Committed:
https://chromium.googlesource.com/chromium/src/+/fe045f9d49ebba8328b0fd7abca5...
==========

[commit-bot: I haz the power, 2017-03-29 13:49:31]

Committed patchset #4 (id:50001) as
https://chromium.googlesource.com/chromium/src/+/fe045f9d49ebba8328b0fd7abca5...