Make report-non-inclusive-language ignore following directories and files: 1) 'autoinstalled': The source code of packages inside autoinstalled directories (e.g.: Scripts/libraries/autoinstalled) are not part of our repository and is not controlled by us. So report-non-inclusive-language shouldn't count them. Currently the count is really skewed because of counting such autoinstalled directories. 2) Files ending with .swp: usually these are files created for files opened in other tabs). 3) Files ending with .log: usually these are not part of source code, and are simply logs. For e.g.: twistd.log while running buildbot instance locally.
Created attachment 411887 [details] Patch
It should also ignore files ending in .pyc
Created attachment 411892 [details] Patch
Comment on attachment 411887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411887&action=review > Tools/Scripts/report-non-inclusive-language:47 > +IGNORE_FILES_ENDING_WITH = ('.order', '.xcuserstate', '.swp', '.log') I saw your comment about adding .pyc here, but R+ for now anyway. This is great!
Committed r268752: <https://trac.webkit.org/changeset/268752> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411892 [details].
<rdar://problem/70496351>
Comment on attachment 411892 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411892&action=review > Tools/Scripts/report-non-inclusive-language:61 > + if any(directory in prefix for directory in IGNORE_DIRECTORIES): This is IGNORE_DIRECTORIES or IGNORE_DIRECTORIES_CONTAINING? It looks like we are using substring matching for directory names, but we want to know if this is exactly a directory name. Ideally this should not match "notautoinstalled". I’d like a more correct implementation to be future-looking although for now none of those three strings seems like a problem. I believe that some others at Apple have been using this script, so it’s good if it works well even outside WebKit, which is why it’s good to be careful about such things.
(In reply to Darin Adler from comment #7) > It looks like we are using substring matching for directory names, but we want to know if this is exactly a directory name. Ideally this should not match "notautoinstalled". Good point. Should we do the exact match for the directories. That seems like the right thing to do. Proposed patch would be something like: - if any(directory in prefix for directory in IGNORE_DIRECTORIES): + if any(directory==prefix for directory in IGNORE_DIRECTORIES):
(In reply to Aakash Jain from comment #8) > (In reply to Darin Adler from comment #7) > > It looks like we are using substring matching for directory names, but we want to know if this is exactly a directory name. Ideally this should not match "notautoinstalled". > Good point. Should we do the exact match for the directories. That seems > like the right thing to do. Proposed patch would be something like: > > - if any(directory in prefix for directory in IGNORE_DIRECTORIES): > + if any(directory==prefix for directory in IGNORE_DIRECTORIES): Ignore my above comment, just noticed that prefix contains the complete path.
Reopening to attach minor follow-up patch.
Created attachment 411908 [details] Patch
Comment on attachment 411908 [details] Patch For now, this is OK.
(In reply to Aakash Jain from comment #8) > (In reply to Darin Adler from comment #7) > > It looks like we are using substring matching for directory names, but we want to know if this is exactly a directory name. Ideally this should not match "notautoinstalled". > Good point. Should we do the exact match for the directories. That seems > like the right thing to do. Proposed patch would be something like: > > - if any(directory in prefix for directory in IGNORE_DIRECTORIES): > + if any(directory==prefix for directory in IGNORE_DIRECTORIES): Maybe: if any(directory.endsWith("/" + prefix) for directory in IGNORE_DIRECTORIES): Would that work?
Committed r268763: <https://trac.webkit.org/changeset/268763> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411908 [details].
(In reply to Darin Adler from comment #13) > Maybe: > > if any(directory.endsWith("/" + prefix) for directory in > IGNORE_DIRECTORIES): > > Would that work? That doesn't seem to work. Uploaded patch in https://bugs.webkit.org/show_bug.cgi?id=217993 which seems to work fine.