chore: renamed non-inclusive language #18319

Gilbert09 · 2023-11-01T10:42:46Z

Problem

We're using non-inclusive language within our codebase, using terms such as whitelist and blacklist to describe good/bad lists of items.
As described here https://github.com/PostHog/company-internal/issues/1191 - I'd like us to change the language we write in code to be more inclusive

Changes

Adds the woke linter to CI to warn us of any usages of non-inclusive language, and provide alternatives
Updates most of our current usage of non-inclusive language
- Some places are not possible to change just yet, for example, we're using the term whitelist in a database field, and so this requires a database migration to push through - to come later

posthog-bot · 2023-11-01T10:43:00Z

Hey @Gilbert09! 👋
This pull request seems to contain no description. Please add useful context, rationale, and/or any other information that will help make sense of this change now and in the distant Mars-based future.

pauldambra · 2023-11-01T10:46:36Z

Can't review today but strong plus 1

daibhin · 2023-11-01T11:25:40Z

plugin-server/src/worker/ingestion/event-pipeline/runner.ts

                eventDroppedCounter
                    .labels({
                        event_type: 'analytics',
-                        drop_cause: 'blacklisted',
+                        drop_cause: 'disallowed',


Could this impact analytics that are relied on?

Great question.

@tiina303 Git blame tells me you were the last around these parts of the code. Do you know whether there are any implications for changing this?

ooh indeed great catch, it is changing the metric label, so yes filtering for it in grafana-prod-us would break / need updating

All good with this one. For any metrics I'd just search in the charts repo, where we have dashboards/alerts https://github.com/search?q=repo%3APostHog%2Fcharts%20blacklisted&type=code

Sorry, I'm rather new with Grafana and I'm not too sure what I'm looking for in there. Considering the search in the charts repo returns nothing for me, are we all good, or are those two things not connected?

yes the search returns nothing (for neither repo), so we don't use the label explicitly in alerts nor dashboards.
example alerts conf: https://github.com/PostHog/charts/blob/main/config/vmalert/alerts/pipeline.yml
example dashboard conf: https://github.com/PostHog/posthog-cloud-infra/blob/fb2a10888e5c8ac2cb7c2268f1c710e38e49c9f2/terraform/modules/grafana/dashboards/webhooks.json#L1165

plugin-server/src/worker/ingestion/event-pipeline/runner.ts

neilkakkar · 2023-11-01T13:22:47Z

plugin-server/src/worker/ingestion/event-pipeline/runner.ts

                eventDroppedCounter
                    .labels({
                        event_type: 'analytics',
-                        drop_cause: 'blacklisted',
+                        drop_cause: 'disallowed',


ooh indeed great catch, it is changing the metric label, so yes filtering for it in grafana-prod-us would break / need updating

Co-authored-by: Neil Kakkar <[email protected]>

mariusandra

I'm fine with the changes, but I disagree with adding an extra tool (linter) to watch over us.

We've said over and over that process is the scar tissue of an organisation, and this is akin to adding more process because someone did something wrong in the past. We have been really good at flagging new instances of non inclusive language, so I would strongly consider removing the woke linter, fix existing issues, and trust that the current review process is good enough. It has been so far, at least in recent memory.

Gilbert09 · 2023-11-01T14:48:45Z

I'm fine with the changes, but I disagree with adding an extra tool (linter) to watch over us.

We've said over and over that process is the scar tissue of an organisation, and this is akin to adding more process because someone did something wrong in the past. We have been really good at flagging new instances of non inclusive language, so I would strongly consider removing the woke linter, fix existing issues, and trust that the current review process is good enough. It has been so far.

@mariusandra I'd challenge this and say that the current review process already isn't working considering that we have 202 cases of non-inclusive language in this one codebase - some of which was added in the last few months. I understand the idea of process being the scar tissue of an organisation, but the "process" in this scenario should be minimal. The linter won't fail and block PRs if it finds examples of the disallowed language, but instead, just annotate the code with a reminder and suggest an alternative. This can of course be ignored if need be, but I feel like this is a friendly low-maintenance way of avoiding any non-inclusive language accidentally entering the codebase.

That being said, if you feel strongly against this, then I'm happy to remove the CI linter and will just keep an eye on this myself over the coming months and revisit it down the line if need be.

posthog-bot · 2023-11-01T14:50:25Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

pauldambra · 2023-11-01T15:09:07Z

IMHO "think about this when you review a PR" is more process than "add a linter and forget about it"

I'd vote let's leave the linter in and if it gets in the way we can revisit it.

I think the linter name is hilarious but "woke" as a term has been pretty weaponised so not everyone would agree. If there's an alternative linter with a different name I'd be tempted to say we should use that but that's not a strong suggestion

pauldambra · 2023-11-01T15:10:28Z

The other argument for a linter is that English isn't everyone's first language. Forcing folk to think in their second (or third/fourth/fifth you show-offs) language and remember nuance of the changing acceptability of different terms is more work than letting the computer do it

posthog-bot · 2023-11-01T15:15:51Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-11-01T15:41:27Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 0 modified, 0 deleted
webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

corywatilo · 2023-11-01T18:22:22Z

Agreed with Marius that the sentiment is fine, though some of this feels just a little silly. Not being able to say "sanity check" or "dummy data" feels like this goes a little too far. No one in their right mind should be offended by these phrases, and if they do, that's quite frankly not our problem, and we shouldn't have to dance around words.

I also worry about the appearance issues of having something called "woke" in the codebase as this is a huge red flag that we prioritize wokeness over just building a good product. When the two intersect, it very often leads to organizational bloat that prioritizes political correctness above everything else. And for someone like me who is here to put my head down and build a great product, it would make me think twice about joining.

TL;DR: Sentiment is great, but we should carefully consider the repercussions of going too far, as there is a (much larger) swath of people who will be turned off by this more than the benefits of trying to be more gentle with our language.

…itelisting

Gilbert09 · 2023-11-01T18:54:57Z

Feels like there's a common theme from the pushback here, and so to find some middle ground, I've removed the linting and reverted the changes that didn't involve the terms blacklisting/whitelisting - as these are terms we 100% shouldn't be using.
Hopefully, this gives us a solid foundation to build upon.

For extra context, the use of both "dummy" and "sanity check" has been associated with something known as ableist language. Taken from here, this is a good summary of it:

This is a concept that seems less widely understood than other expressions of bias, like racist language or sexist language. But it’s really quite simple: Ableist language presents disabled people as inherently problematic, incompetent, or simply having less value. It often dehumanizes or marginalizes disabled people.

Gilbert09 · 2023-11-01T18:55:39Z

.wokeignore

+.wokeignore
+posthog/management/sql/0050_event_partitions.sql
+posthog/migrations/0001_initial_squashed_0284_improved_caching_state_idx.py
+posthog/migrations/0160_organization_domain_whitelist.py
+posthog/migrations/0161_property_defs_search.py
+posthog/migrations/0223_organizationdomain.py
+*.ambr
+frontend/src/scenes/plugins/source/types/packages.json
+# External lib settings
+posthog/settings/web.py
+# External lib settings 
+ee/settings.py


I'm planning on keeping this in for the time being, it makes checking the code base for any future regressions much easier for me

posthog-bot · 2023-11-01T19:23:57Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

jamesefhawkins · 2023-11-01T23:21:51Z

I think your middle ground is good here @Gilbert09

mariusandra · 2023-11-02T07:14:07Z

Thanks for dialing down the changes to the least controversial ones. I completely agree we should stop using these two racially motivated words, so thanks for taking this on!

However I would still remove the .wokeignore file. Like both Paul and Cory said, the term "woke" is overloaded/weaponized, and I have never heard it used in a context where it's not a slur. Seeing a unused file (effectively commented out code), with a polarizing slur word in the root of the repo will trigger those negative connotations Cory refers to in a sizable amount of people, including myself. If we truly want to be inclusive, we should paradoxically also watch our language here. 🤔

Gilbert09 · 2023-11-02T10:43:56Z

Thank you everyone for your feedback. Feels like we're coming to a nice middle-ground conclusion here. For what it's worth, I have strong opinions on the usage of racially motivated words, but a much weaker opinion on the extra terms. My original approach here was to use third-party software built by people who have given this problem a lot more thought than myself (e.g. woke in this circumstance).

@mariusandra In terms of the .wokeignore file. I've instead added this to .gitignore so that I can keep it in my filesystem and re-use it at a later date. I hope this satisfies the unused file with a polarizing slur as the name problem you mentioned above. I'd still like to do spot checks in the coming months to check the health of the codebase and having this file around, at least in my own filesystem, will make the process for myself a lot easier.

mariusandra

This is good for me! Thanks for keeping us accountable around these issues! 👍

Renamed non-inclusive language

9b69aeb

Gilbert09 changed the title ~~Renamed non-inclusive language~~ chore: renamed non-inclusive language Nov 1, 2023

daibhin reviewed Nov 1, 2023

View reviewed changes

neilkakkar reviewed Nov 1, 2023

View reviewed changes

Update plugin-server/src/worker/ingestion/event-pipeline/runner.ts

2ef228c

Co-authored-by: Neil Kakkar <[email protected]>

mariusandra requested changes Nov 1, 2023

View reviewed changes

Update UI snapshots for chromium (2)

a9973b7

Update UI snapshots for webkit (2)

119900b

Update UI snapshots for chromium (2)

083ba77

Update UI snapshots for webkit (2)

054f820

Remove woke linting and revert any changes that arent blacklisting/wh…

d24c29f

…itelisting

Gilbert09 commented Nov 1, 2023

View reviewed changes

Update UI snapshots for chromium (2)

03842a3

Added wokeignore to git ignore

5c8c545

mariusandra approved these changes Nov 2, 2023

View reviewed changes

Merge branch 'master' into tom/inclusion

e5db976

github-actions bot added 2 commits November 2, 2023 11:23

Update query snapshots

fcd6346

Update query snapshots

d069d2f

Gilbert09 merged commit 23ddea1 into master Nov 2, 2023
74 checks passed

Gilbert09 deleted the tom/inclusion branch November 2, 2023 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: renamed non-inclusive language #18319

chore: renamed non-inclusive language #18319

Gilbert09 commented Nov 1, 2023 •

edited

Loading

posthog-bot commented Nov 1, 2023

pauldambra commented Nov 1, 2023

daibhin Nov 1, 2023

Gilbert09 Nov 1, 2023

neilkakkar Nov 1, 2023

tiina303 Nov 1, 2023

Gilbert09 Nov 1, 2023

tiina303 Nov 1, 2023

neilkakkar Nov 1, 2023

mariusandra left a comment •

edited

Loading

Gilbert09 commented Nov 1, 2023

posthog-bot commented Nov 1, 2023

pauldambra commented Nov 1, 2023

pauldambra commented Nov 1, 2023

posthog-bot commented Nov 1, 2023

posthog-bot commented Nov 1, 2023

corywatilo commented Nov 1, 2023

Gilbert09 commented Nov 1, 2023

Gilbert09 Nov 1, 2023

posthog-bot commented Nov 1, 2023

jamesefhawkins commented Nov 1, 2023

mariusandra commented Nov 2, 2023 •

edited

Loading

Gilbert09 commented Nov 2, 2023

mariusandra left a comment

chore: renamed non-inclusive language #18319

chore: renamed non-inclusive language #18319

Conversation

Gilbert09 commented Nov 1, 2023 • edited Loading

Problem

Changes

posthog-bot commented Nov 1, 2023

pauldambra commented Nov 1, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariusandra left a comment • edited Loading

Choose a reason for hiding this comment

Gilbert09 commented Nov 1, 2023

posthog-bot commented Nov 1, 2023

📸 UI snapshots have been updated

pauldambra commented Nov 1, 2023

pauldambra commented Nov 1, 2023

posthog-bot commented Nov 1, 2023

📸 UI snapshots have been updated

posthog-bot commented Nov 1, 2023

📸 UI snapshots have been updated

corywatilo commented Nov 1, 2023

Gilbert09 commented Nov 1, 2023

Choose a reason for hiding this comment

posthog-bot commented Nov 1, 2023

📸 UI snapshots have been updated

jamesefhawkins commented Nov 1, 2023

mariusandra commented Nov 2, 2023 • edited Loading

Gilbert09 commented Nov 2, 2023

mariusandra left a comment

Choose a reason for hiding this comment

Gilbert09 commented Nov 1, 2023 •

edited

Loading

mariusandra left a comment •

edited

Loading

mariusandra commented Nov 2, 2023 •

edited

Loading