Exclude ChromeOS directories from Mac coverage lines....

Exclude ChromeOS directories and files from mac win linux.

Extension of this change, applied more liberally:
http://src.chromium.org/viewvc/chrome/trunk/src/build/linux/chrome_linux.croc?r1=44710&r2=46647


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=61430

[joth, 2010-09-30 08:35:22]

lgtm

Thanks!

[bulach, 2010-09-30 09:18:09]

LGTM

fantastic, thanks John! :)
not sure how the regexp work in this context, but we have some files named
chromeos, not in a directory /chromeos/... should we move our files?

slightly unrelated but whilst on mac coverage:
We're running browser_tests on Linux (see *_browsertest.cc):
http://build.chromium.org/buildbot/coverage/linux-debug/60946/CHROMIUM/chrome...

...but apparently not on Mac.
http://build.chromium.org/buildbot/coverage/mac-debug/60906/CHROMIUM/chrome/b...

would it be possible to include them as well, or are they intentionally
excluded?

many thanks once again!

[joth, 2010-09-30 09:28:57]

On 30 September 2010 10:18, <bulach@chromium.org> wrote:

> LGTM
>
> fantastic, thanks John! :)
> not sure how the regexp work in this context, but we have some files named
> chromeos, not in a directory /chromeos/... should we move our files?
>
> Good point. I'm sure we don't want to move these files --  it would be
completely inconsistent with the other platform files.
Couldn't chromeos just be added to the exclusion set on line 11? (i.e.
in (linux|win|views)
)


Looking at the linked file, I guess this means linux has the same issue.
(It's pulling in _chromeos.* files even though it's not running them)

[John Grabowski, 2010-10-01 23:43:26]

Made more inclusive; applies to both files and directories (e.g. *_chromeos.*
and */chromeos/*).

Apply to all platforms.

Please re-review.

[bulach, 2010-10-04 09:42:04]

LGTM

great stuff, thanks John! one suggestion below, not sure if valid:

http://codereview.chromium.org/3540002/diff/6001/7002
File build/mac/chrome_mac.croc (right):

http://codereview.chromium.org/3540002/diff/6001/7002#newcode16
build/mac/chrome_mac.croc:16: 'regexp' : '.*/chromeos/',
you may want to add this rule to chrome_linux.croc, or perhaps change the regexp
above to add trailing slash and remove this entirely:
.*(_|/)(chromeos|linux|win|views)(\\.|_|/)

[John Grabowski, 2010-10-04 20:21:39]

http://codereview.chromium.org/3540002/diff/6001/7002
File build/mac/chrome_mac.croc (right):

http://codereview.chromium.org/3540002/diff/6001/7002#newcode16
build/mac/chrome_mac.croc:16: 'regexp' : '.*/chromeos/',
On 2010/10/04 09:42:05, bulach wrote:
> you may want to add this rule to chrome_linux.croc, or perhaps change the
regexp
> above to add trailing slash and remove this entirely:
> .*(_|/)(chromeos|linux|win|views)(\\.|_|/)

I would prefer to have 2 regexps which are marginally simpler.
E.g. one for filenames, one for directories.
regexps are hard enough to parse as it is.

Note chrome_linux.croc excludes both /chromeos/ and *_chromeos* with this CL. 
(But linux also ignores views directories, so the regexp doesn't match exactly.)

Thanks for the scrutiny.

[John Grabowski, 2010-10-04 23:00:28]

I've landed this.  It may take a little while to cycle in (depends on lkgr
changes, but it could be days).
Please help me keep an eye on things in case this chokes the bots.

jrg


On Mon, Oct 4, 2010 at 1:21 PM, <jrg@chromium.org> wrote:

>
> http://codereview.chromium.org/3540002/diff/6001/7002
> File build/mac/chrome_mac.croc (right):
>
> http://codereview.chromium.org/3540002/diff/6001/7002#newcode16
> build/mac/chrome_mac.croc:16: 'regexp' : '.*/chromeos/',
> On 2010/10/04 09:42:05, bulach wrote:
>
>> you may want to add this rule to chrome_linux.croc, or perhaps change
>>
> the regexp
>
>> above to add trailing slash and remove this entirely:
>> .*(_|/)(chromeos|linux|win|views)(\\.|_|/)
>>
>
> I would prefer to have 2 regexps which are marginally simpler.
> E.g. one for filenames, one for directories.
> regexps are hard enough to parse as it is.
>
> Note chrome_linux.croc excludes both /chromeos/ and *_chromeos* with
> this CL.  (But linux also ignores views directories, so the regexp
> doesn't match exactly.)
>
> Thanks for the scrutiny.
>
>
> http://codereview.chromium.org/3540002/show
>