Bug 215156 - Make report-non-inclusive-language ignore files within .svn and .git
Summary: Make report-non-inclusive-language ignore files within .svn and .git
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-04 22:11 PDT by Beth Dakin
Modified: 2020-10-20 10:54 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.84 KB, patch)
2020-08-04 22:14 PDT, Beth Dakin
bdakin: review-
Details | Formatted Diff | Diff
Patch (1.88 KB, patch)
2020-08-05 13:07 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch (1.87 KB, patch)
2020-08-05 13:18 PDT, Beth Dakin
darin: review+
Details | Formatted Diff | Diff
Patch (1.77 KB, patch)
2020-08-05 13:27 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2020-08-04 22:11:03 PDT
Make report-non-inclusive-language ignore files within .svn and .git
Comment 1 Beth Dakin 2020-08-04 22:14:15 PDT
Created attachment 405985 [details]
Patch

I'm not sure if this is the best way to do this…
Comment 2 Darin Adler 2020-08-05 11:39:38 PDT
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.
Comment 3 Beth Dakin 2020-08-05 11:57:51 PDT
Comment on attachment 405985 [details]
Patch

Going to rewrite this for sure, so R- for now.
Comment 4 Beth Dakin 2020-08-05 12:46:15 PDT
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"):
Comment 5 Darin Adler 2020-08-05 12:50:28 PDT
OK, then land something that works, and we will figure this out later.
Comment 6 Beth Dakin 2020-08-05 13:07:45 PDT
Created attachment 406029 [details]
Patch

Here's a patch the moves the directory check outside of the files loop.
Comment 7 Darin Adler 2020-08-05 13:09:58 PDT
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:
Comment 8 Beth Dakin 2020-08-05 13:18:36 PDT
Created attachment 406030 [details]
Patch

Oh yeah, I forgot that part of the feedback. Thank you!
Comment 9 Darin Adler 2020-08-05 13:23:52 PDT
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?
Comment 10 Beth Dakin 2020-08-05 13:27:37 PDT
Created attachment 406031 [details]
Patch

Fixing the sloppy!
Comment 11 EWS 2020-08-05 14:08:10 PDT
Committed r265304: <https://trac.webkit.org/changeset/265304>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406031 [details].
Comment 12 Radar WebKit Bug Importer 2020-08-05 14:09:18 PDT
<rdar://problem/66593010>