Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor out woke instructions #158

Merged
merged 9 commits into from
Jan 4, 2024

Conversation

pmatulis
Copy link

@pmatulis pmatulis commented Dec 13, 2023

Preamble: The readme.rst file in this repository is way too long. It also covers both setup and regular maintenance duties, making it hard for a contributor to find what they need once a repository has been set up by an administrator. This PR is the first of a suite of PRs for addressing the situation.

Move the details of working with inclusive language exemptions into a separate file. In so doing, the text was improved. Remove syntax highlighting for the reST code blocks as they make things look ugly and alien-like:

image

Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with splitting it out if you think that improves the user experience.
However, I'm not a big fan of it because it means more content files that users need to remove from the starter pack before they can add their own content.

help-woke.rst Outdated
@@ -0,0 +1,46 @@
:orphan:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use orphans, but include the file in the navigation. There's no reason to hide it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this if you feel strongly about it but the resource is intended to be accessed in the context of the README. It's a sub-page.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it can be a subpage to the readme. But I feel pretty strongly about it that all pages should always be accessible from the navigation tree - otherwise it's very easy to miss that there is another page with information.

help-woke.rst Outdated
-------------

To exempt an individual word, place a comment on a line immediately preceding
the line containing the word in question. This special comment is to include
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
the line containing the word in question. This special comment is to include
the line containing the word in question. This special comment must include

Easier to understand

help-woke.rst Outdated
Comment on lines 23 to 24
Here is an example of an exemption that acts upon an element of a URL that is
expressed using the link definition method (typically at the bottom of a file):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find that clearer - will everyone who uses the starter pack know what the "link definition method" is?

The issue that we see a lot is that a URL contains "master", and that this is hard to fix with a comment in the line above (especially in MyST, btw). So what we should say is that for links, the best solution is to define them in a separate file or at the bottom of the page and put the comment in the line above the link definition.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will propose something.

help-woke.rst Outdated
A more drastic solution is to make an exemption for the contents of an entire
file.

Start by placing file ``.wokeignore`` into your project's root directory. Then,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The starter pack already has this file, so you only need to edit it.

readme.rst Outdated
change how the ``woke`` tool is invoked from within ``docs/Makefile`` (see
the `woke User Guide <https://docs.getwoke.tech/usage/#file-globs>`_ for help).

Some circumstances may compel you to retain some non-inclusive words. In such
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some circumstances may compel you to retain some non-inclusive words. In such
Some circumstances might require you to use some non-inclusive words. In such

Let's keep it simple. ;)

Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'm happy with the text now. 🙂
But need to fix the woke test.

@pmatulis
Copy link
Author

Did I miss something?

@ru-fu
Copy link
Contributor

ru-fu commented Jan 2, 2024

Did I miss something?

The doc check on the PR is red - because you have a "master" in the text without an exception.

help-woke.rst Outdated
This is your text. The word in question is here: whitelist. More text.

Here is an example of an exemption that acts upon an element (the string
wokeignore:rule=master
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work - it'll leave a wokeignore:rule=master both in the GitHub rendering and in the HTML output generated from RST.
I just tested this again... RST support for wokeignore is horrible. It doesn't work as described in line 18.
Let me figure something out ...

@ru-fu
Copy link
Contributor

ru-fu commented Jan 2, 2024

OK ... I couldn't find anything that doesn't feel like a hack, but this solution seems okay-ish to me:

Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should now work to put a :none:`wokeignore:rule=master,` at the end of a line that contains a violation.
You need to include the , at the end because otherwise woke doesn't seem to parse it correctly (don't ask me why ...). And putting it in the line above also doesn't work; I haven't managed to find a workaround for that so far.

help-woke.rst Outdated
Comment on lines 18 to 24
.. wokeignore:rule=whitelist
This is your text. The word in question is here: whitelist. More text.

Here is an example of an exemption that acts upon an element (the string
wokeignore:rule=master
"master") of a URL. It is recommended to do this by using the standard reST
method of placing the link at the bottom of the page (or in a separate file):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. wokeignore:rule=whitelist
This is your text. The word in question is here: whitelist. More text.
Here is an example of an exemption that acts upon an element (the string
wokeignore:rule=master
"master") of a URL. It is recommended to do this by using the standard reST
method of placing the link at the bottom of the page (or in a separate file):
This is your text. The word in question is here: whitelist. More text. :none:`wokeignore:rule=whitelist,`
Here is an example of an exemption that acts upon an element (the string
"master") of a URL. It is recommended to do this by using the standard reST :none:`wokeignore:rule=master,`
method of placing the link at the bottom of the page (or in a separate file):

Peter Matulis added 8 commits January 3, 2024 12:08
Signed-off-by: Peter Matulis <[email protected]>
Signed-off-by: Peter Matulis <[email protected]>
Signed-off-by: Peter Matulis <[email protected]>
Signed-off-by: Peter Matulis <[email protected]>
Signed-off-by: Peter Matulis <[email protected]>
Signed-off-by: Peter Matulis <[email protected]>
Signed-off-by: Peter Matulis <[email protected]>
@pmatulis pmatulis force-pushed the refactor-out-woke-instructions branch from 2988244 to 7553d1d Compare January 3, 2024 17:14
Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two more nitpicks, but otherwise it seems to work now. :)

.wordlist.txt Outdated
@@ -44,3 +44,4 @@ UI
UUID
VM
YAML
wokeignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed anymore.

help-woke.rst Outdated

You can now refer to the label ``link definition_`` in the body of the text.

Here :none:`wokeignore:rule=master,` is an example where a URL element contains the string "master":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate of line 35.

Signed-off-by: Peter Matulis <[email protected]>
Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ru-fu ru-fu merged commit df2460d into canonical:main Jan 4, 2024
2 checks passed
@pmatulis pmatulis deleted the refactor-out-woke-instructions branch January 4, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants