Add check for non-inclusive language #44
Conversation
|
@nhosoi once the 'sanity' naming and smaller items are fixed (as you've done) there seems to be two main problems: the use of 'whitelist' in the grafana config file (based on upstream) and 'master' in the redis config (also from upstream) - these are things we can't change and diverging from upstream configs is not ideal. Can we exclude these two classes of upstream files from checking at the file level rather than the word level? I think this will prove more maintainable in the longer term. |
woke doesn't give us that level of granularity. If you use .wokeignore to exclude a file, it will exclude that file from checking for all terms, not just specific terms. e.g. if we add the grafana config files to .wokeignore because we want to allow |
That's fine though - it's exactly what we want because we have no control over the contents of these files, they're from upstream Redis and Grafana projects. |
|
@natoscott, @richm, could there be anything else we should update this pr to make sure the inclusive language is ensured (=~ |
Well, not exactly. The files https://github.com/performancecopilot/ansible-pcp/blob/main/roles/grafana/templates/grafana_7.ini.j2 and https://github.com/performancecopilot/ansible-pcp/blob/main/roles/grafana/templates/grafana_9.ini.j2 are templates, not "plain" files, which means you can't just use the raw files from grafana upstream - someone on the pcp team must edit them to insert the jinja constructs used. So if you are editing them anyway, you could add the wokeignore directives. OTOH, it sure would be nice to have the ability to allow certain terms in certain files, that could be specified in .wokeignore or .woke.yml So I guess for now we can just add the templates to .wokeignore and do no scanning of them |
|
@richm yep, understood re templates and them not being exactly the same as upstream already. Each change we make to them though makes maintenance slightly more involved when we move forward to new versions. From a practical point of view I wonder if explicitly calling out terms (via inline markup) that we cannot resolve is even more damaging than just quietly ignoring the file and not drawing attention to the problematic phrasing. shrug | So I guess for now we can just add the templates to .wokeignore and do no scanning of them +1 - I think this is the best approach in these two situations (grafana/redis). Anywhere else in PCP & the rest of the metrics role we can definitely just fix and move on. |
|
@nhosoi I'm not aware of any other areas we can improve at this stage. We've independently audited all of PCP and fixed everything we can there. |
|
Thanks for your comments, @natoscott! |
|
Yes please - I think we should treat these files exactly like the Redis ones, its upstream content with known problematic phrasing that we have every reason to expect will continue going forward (which we have no control over). |
- Rename tests/tests_sanity_.*.yml to tests/tests_verify_.*.yml. Signed-off-by: Noriko Hosoi <[email protected]>
Add a check for usage of terms and language that is considered non-inclusive. We are using the woke tool for this with a wordlist that can be found at https://github.com/linux-system-roles/tox-lsr/blob/main/src/tox_lsr/config_files/woke.yml Add .wokeignore to skip checking files in roles/redis/templates/ and roles/grafana/templates/. Signed-off-by: Noriko Hosoi <[email protected]>
|
Done! It's much simpler, now. Thanks, @natoscott! |
Add check for non-inclusive language
Add a check for usage of terms and language that is considered non-inclusive. We are using the woke tool for this with a wordlist that can be found at
https://github.com/linux-system-roles/tox-lsr/blob/main/src/tox_lsr/config_files/woke.yml
Note: this commit uses the customized woke placed locally in .github/actions/custom-woke-action to support a new option --count-only-error-for-failure option. The local action custom-woke-action will be replaced with the official woke once get-woke/woke#252 (Add an option "--count-only-error-for-failure") is processed.
Avoid non-inclusive language
Additional note: There are too many warning level words
masterto skip with# wokeignore:rule=master. So, I use the customized woke to skip the warning words in this PR. The# wokeignore:rule=wordis applied to the error level wordwhitelist.