Bug 215156 – Make report-non-inclusive-language ignore files within .svn and .git

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Beth Dakin]

Make report-non-inclusive-language ignore files within .svn and .git

[Beth Dakin]

Created attachment 405985 [details]
Patch

I'm not sure if this is the best way to do this…

[Darin Adler]

Comment on attachment 405985 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405985&action=review

> Tools/Scripts/report-non-inclusive-language:69
> +        if prefix.find(".svn") >= 0:
> +            continue
> +        if prefix.find(".git") >= 0:
> +            continue

1. I suggest doing before the "for file in files:" loop, since it’s a check on the entire directory, not individual files.

2. I found documentation here <https://docs.python.org/2/library/stdtypes.html#string-methods> that says you can and should write this instead:

    if ".svn" in prefix:
        continue;

3. (This seems to supersede both 1 and 2 above.)

II did a little research and found a different solution, a way to make os.walk not even recurse into these directories. We can write this in the os.walk loop (outside the "for file in files" loop):

    directories = [directory for directory in directories if not directory.startswith(".svn")]

Then it will never even recurse into a that starts with .svn. Maybe there’s a nicer way to write it if you are a Python expert.

[Beth Dakin]

Comment on attachment 405985 [details]
Patch

Going to rewrite this for sure, so R- for now.

[Beth Dakin]

I might be being too much of a Python n00b here, but this patch did not work for me. .svn files were still traversed:

Index: Scripts/report-non-inclusive-language
===================================================================
--- Scripts/report-non-inclusive-language	(revision 265267)
+++ Scripts/report-non-inclusive-language	(working copy)
@@ -54,6 +55,7 @@
 
 root = os.getcwd()
 for subroot, directories, files in os.walk(root):
+    directories = [directory for directory in directories if not directory.startswith(".svn")]
     prefix = subroot[len(root) + 1:]
     for file in files:
         if file.startswith("ChangeLog"):

[Darin Adler]

OK, then land something that works, and we will figure this out later.

[Beth Dakin]

Created attachment 406029 [details]
Patch

Here's a patch the moves the directory check outside of the files loop.

[Darin Adler]

Comment on attachment 406029 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406029&action=review

> Tools/Scripts/report-non-inclusive-language:58
> +    directories = [directory for directory in directories if not directory.startswith(".svn")]

Leave this out since it's not working yet

> Tools/Scripts/report-non-inclusive-language:63
> +    if prefix.find(".svn") >= 0:
> +        continue
> +    if prefix.find(".git") >= 0:
> +        continue

Use this style:

    if ".svn" in prefix:

[Beth Dakin]

Created attachment 406030 [details]
Patch

Oh yeah, I forgot that part of the feedback. Thank you!

[Darin Adler]

Comment on attachment 406030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406030&action=review

> Tools/Scripts/report-non-inclusive-language:58
> +    directories = [directory for directory in directories if not directory.startswith(".svn")]

Take this out since it’s not working?

[Beth Dakin]

Created attachment 406031 [details]
Patch

Fixing the sloppy!

[EWS]

Committed r265304: <https://trac.webkit.org/changeset/265304>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406031 [details].

[Radar WebKit Bug Importer]

<rdar://problem/66593010>