Add check for non-inclusive language #142

nhosoi · 2022-12-13T02:36:54Z

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. It will be replaced with
the official woke once get-woke/woke#252
(Add an option "--count-only-error-for-failure") is processed.

CHANGELOG.md - cleanup non-inclusive words.
tests/tasks/setup_ipa.yml - Apply "wokeignore:rule"

nhosoi · 2022-12-13T02:38:05Z

Note: If 38a2f3a is acceptable, we can use the action woke-action with the released woke.

nhosoi · 2022-12-13T05:27:34Z

[citest]

richm · 2022-12-13T22:37:43Z

.github/workflows/woke.yml

+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2


Suggested change

uses: actions/checkout@v2

uses: actions/checkout@v3

nhosoi · 2022-12-15T19:18:45Z

[citest]

richm · 2022-12-15T19:25:06Z

tests/tasks/setup_ipa.yml

@@ -12,6 +12,7 @@
 - name: Clone ansible-freeipa repo
  git:
    repo: https://github.com/freeipa/ansible-freeipa
+    # wokeignore:rule=master
    version: master


Here you can omit version: master - by default it will grab whatever branch is configured to be the default branch

ansible-lint reports this error if no version: master is specified.

Error: git-latest Git checkouts must contain explicit version You can skip specific rules or tags by adding them to your configuration file: # .ansible-lint warn_list: # or 'skip_list' to silence them completely - git-latest # Git checkouts must contain explicit version Finished with 1 failure(s), 0 warning(s) on 82 files. ERROR: InvocationError for command /home/runner/work/certificate/certificate/.tox/ansible-lint/bin/ansible-lint -v --exclude=tests/roles -c .ansible-lint (exited with code 2)

Which do we prefer between having version: master preceded by # wokeignore:rule=master or adding - '403' to the skip-list in .ansible-lint?

This commit 604afea takes the latter.

I would prefer noqa latest - https://ansible-lint.readthedocs.io/rules/latest/

# noqa latest git: repo: https://github.com/freeipa/ansible-freeipa

I much, much prefer putting such suppressions inline in the code rather than the config file

the suppression is next to the problematic code

easier to deal with conversion to collection - don't have to merge a bunch of .ansible-lint files together

doesn't affect all files

(/me wishes the error message said this rather than "just add it to the .ansible-lint file" :-(

Please ignore what I wrote previously. It worked!

@@ -10,6 +10,7 @@ when: __is_beaker_env - name: Clone ansible-freeipa repo + # noqa git-latest git: repo: https://github.com/freeipa/ansible-freeipa dest: /tmp/freeipa-repo

And I ran tox -e ansible-lint.

______________________________________________ summary ______________________________________________ ansible-lint: commands succeeded congratulations :)

richm · 2022-12-15T19:25:58Z

So if you omit the version: master from the setup_ipa, and edit the CHANGELOG.md to remove references to sanity, can you use the official woke github action instead of the custom one?

nhosoi · 2022-12-15T19:31:30Z

So if you omit the version: master from the setup_ipa, and edit the CHANGELOG.md to remove references to sanity, can you use the official woke github action instead of the custom one?

Yes, we can. Do we want to avoid using ansible-test sanity? Just say ansible-test?

richm · 2022-12-15T20:09:26Z

So if you omit the version: master from the setup_ipa, and edit the CHANGELOG.md to remove references to sanity, can you use the official woke github action instead of the custom one?

Yes, we can. Do we want to avoid using ansible-test sanity? Just say ansible-test?

Yes, just say ansible-test

nhosoi · 2022-12-16T00:37:57Z

[citest]

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 Signed-off-by: Noriko Hosoi <nhosoi@redhat.com>

richm · 2022-12-16T03:05:44Z

tests/tasks/setup_ipa.yml

@@ -53,6 +53,7 @@
  rescue:
    - name: FAILURE - check entropy
      command: cat /proc/sys/kernel/random/entropy_avail
+      changed_when: false


don't worry about these - we'll change these later when we make all of the roles work with ansible-lint 6.x - looks like we have some ansible-core 2.14 work to do as well - warn: false breaks with 2.14

CHANGELOG.md,tests/tasks/setup_ipa.yml - Cleanup non-inclusive words.

rafasgj · 2022-12-16T12:21:33Z

About the 'version: master' on ansible-freeipa checkout, since I lost the argument to change it to "main", as we use it only to deploy IPA, and there won't be many changes, specially on what we need, we could pin the version to 'v1.9.0", and avoid the use of "master".

rafasgj · 2022-12-16T13:33:40Z

tests/tasks/setup_ipa.yml

  git:
    repo: https://github.com/freeipa/ansible-freeipa
-    version: master


Here's what I meant about the use of 'master'. We can safely pin version 'v1.9.0' and update it if we need in the future.

On the maintainance cost of updating it, I believe it will be updated soon, but not often.

Why can't we just omit version altogether, then git will use whatever is the default branch?

If we can, it would be better. I though it was required.

It is not required by the git module, but it is by ansible-lint. So, we are adding this line to skip the check by ansible-lint.
https://github.com/linux-system-roles/certificate/pull/142/files#diff-d1ca26ec3f441ccd0c4bb18b601d19668a8ffacfab0c71694fea7b46adc9823cR13

nhosoi · 2022-12-19T21:54:15Z

@richm, @rafasgj, @spetrosi, could you please review this pr one more time? Thanks!

rjeffman

@nhosoi, although I don't understand why we can't pin the ansible freeipa version (and then don't have the # noqa comment, this change looks good to me.

And the use of woke looks promising, specially for the future. Thank you for looking into this issue!

I'm approving these changes.

richm · 2022-12-20T02:51:04Z

@nhosoi, although I don't understand why we can't pin the ansible freeipa version (and then don't have the # noqa comment, this change looks good to me.

My only concern is that it stays on an old version for a long time and never gets updated, which is what has happened in the past:
e84385a
cc7d5a9#diff-d1ca26ec3f441ccd0c4bb18b601d19668a8ffacfab0c71694fea7b46adc9823cL15

So if we have some (preferably automated) process for regularly updating the version, that's fine with me.

And the use of woke looks promising, specially for the future. Thank you for looking into this issue!

I'm approving these changes.

nhosoi · 2022-12-20T05:19:53Z

My only concern is that it stays on an old version for a long time and never gets updated, which is what has happened in the past: e84385a cc7d5a9#diff-d1ca26ec3f441ccd0c4bb18b601d19668a8ffacfab0c71694fea7b46adc9823cL15

So if we have some (preferably automated) process for regularly updating the version, that's fine with me.

Me, too. @rjeffman, do you want me to revert the version: master line? Or any specific version in your mind? And maybe, we'd like to have a tiny github action to check the ansible-freeipa version?

rjeffman · 2022-12-20T11:48:57Z

Me, too. @rjeffman, do you want me to revert the version: master line? Or any specific version in your mind? And maybe, we'd like to have a tiny github action to check the ansible-freeipa version?

Hmm... I have bug to fix where an external dependency is broken. Maybe we could have a Github action that setup all the testing dependencies.

But that's for the future, I'm fine with this PR as it is.

richm · 2023-01-02T17:59:22Z

[citest]

[1.1.8] - 2023-01-20 -------------------- ### New Features - none ### Bug Fixes - ansible-lint 6.x fixes ### Other Changes - Add check for non-inclusive language (linux-system-roles#142) - Cleanup non-inclusive words - ignore files for ansible-test 2.13 and 2.14 (linux-system-roles#149) Signed-off-by: Rich Megginson <rmeggins@redhat.com>

[1.1.8] - 2023-01-20 -------------------- ### New Features - none ### Bug Fixes - ansible-lint 6.x fixes ### Other Changes - Add check for non-inclusive language (#142) - Cleanup non-inclusive words - ignore files for ansible-test 2.13 and 2.14 (#149) Signed-off-by: Rich Megginson <rmeggins@redhat.com> Signed-off-by: Rich Megginson <rmeggins@redhat.com>

nhosoi requested review from richm and rjeffman as code owners December 13, 2022 02:36

nhosoi changed the title ~~Add github action woke.yml (github.com/get-woke/woke-action)~~ [WIP] Add github action woke.yml (github.com/get-woke/woke-action) Dec 13, 2022

richm reviewed Dec 13, 2022

View reviewed changes

nhosoi force-pushed the woke-action branch 2 times, most recently from 424d0d9 to 3cd37a6 Compare December 14, 2022 18:25

nhosoi changed the title ~~[WIP] Add github action woke.yml (github.com/get-woke/woke-action)~~ [WIP] Add check for non-inclusive language Dec 14, 2022

nhosoi force-pushed the woke-action branch from 3cd37a6 to 78b5bf3 Compare December 15, 2022 19:14

nhosoi changed the title ~~[WIP] Add check for non-inclusive language~~ Add check for non-inclusive language Dec 15, 2022

richm reviewed Dec 15, 2022

View reviewed changes

nhosoi force-pushed the woke-action branch from 78b5bf3 to dc66886 Compare December 15, 2022 23:59

nhosoi force-pushed the woke-action branch from 604afea to 9340292 Compare December 16, 2022 01:47

nhosoi force-pushed the woke-action branch from 9340292 to add1323 Compare December 16, 2022 01:50

richm reviewed Dec 16, 2022

View reviewed changes

Cleanup non-inclusive words

f9f9a13

CHANGELOG.md,tests/tasks/setup_ipa.yml - Cleanup non-inclusive words.

nhosoi force-pushed the woke-action branch from add1323 to f9f9a13 Compare December 16, 2022 06:04

rafasgj reviewed Dec 16, 2022

View reviewed changes

rjeffman approved these changes Dec 20, 2022

View reviewed changes

richm merged commit 386ffbf into linux-system-roles:master Jan 3, 2023

richm mentioned this pull request Jan 20, 2023

Version 1.1.8 - CHANGELOG.md [citest skip] #150

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add check for non-inclusive language #142

Add check for non-inclusive language #142

nhosoi commented Dec 13, 2022 •

edited

Loading

nhosoi commented Dec 13, 2022

nhosoi commented Dec 13, 2022

richm Dec 13, 2022

nhosoi commented Dec 15, 2022

richm Dec 15, 2022

nhosoi Dec 16, 2022

nhosoi Dec 16, 2022

richm Dec 16, 2022 •

edited

Loading

nhosoi Dec 16, 2022 •

edited

Loading

richm commented Dec 15, 2022

nhosoi commented Dec 15, 2022

richm commented Dec 15, 2022

nhosoi commented Dec 16, 2022

richm Dec 16, 2022

rafasgj commented Dec 16, 2022

rafasgj Dec 16, 2022

richm Dec 16, 2022

rafasgj Dec 16, 2022

nhosoi Dec 16, 2022

nhosoi commented Dec 19, 2022

rjeffman left a comment •

edited

Loading

richm commented Dec 20, 2022

nhosoi commented Dec 20, 2022

rjeffman commented Dec 20, 2022

richm commented Jan 2, 2023

Add check for non-inclusive language #142

Add check for non-inclusive language #142

Conversation

nhosoi commented Dec 13, 2022 • edited Loading

nhosoi commented Dec 13, 2022

nhosoi commented Dec 13, 2022

Choose a reason for hiding this comment

nhosoi commented Dec 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richm Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

nhosoi Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

richm commented Dec 15, 2022

nhosoi commented Dec 15, 2022

richm commented Dec 15, 2022

nhosoi commented Dec 16, 2022

Choose a reason for hiding this comment

rafasgj commented Dec 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhosoi commented Dec 19, 2022

rjeffman left a comment • edited Loading

Choose a reason for hiding this comment

richm commented Dec 20, 2022

nhosoi commented Dec 20, 2022

rjeffman commented Dec 20, 2022

richm commented Jan 2, 2023

nhosoi commented Dec 13, 2022 •

edited

Loading

richm Dec 16, 2022 •

edited

Loading

nhosoi Dec 16, 2022 •

edited

Loading

rjeffman left a comment •

edited

Loading