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

RFC: Adopt inclusive language for repository naming as well as allow/deny lists #427

Closed
kathytafel opened this issue Nov 22, 2021 · 12 comments
Assignees
Labels

Comments

@kathytafel
Copy link
Contributor

kathytafel commented Nov 22, 2021

Proposal:

"RFC: Rename 'master' branches to 'main' #386" was opened in April and speaks to 1/2 the proposal. We have not yet completed the job. This RFC is to get us over the finish line, with both the master/main topic as well as allow/deny lists. It is a small thing to change language, but with profound impact. Let's not perpetuate the master/slave or 'black = bad' tech metaphors.

Reasoning

All the reasoning in #386.

Exceptions:

Also the exceptions in #386. But no need to update archived repositories.

Additional Context:

As a new joiner, I was surprised when I saw a PR with 'blacklist' because I would have assumed that Artsy had the discussion when that happened in the industry. When asking about it, I was gratified to find out that my assumption was true, by being pointed to this thread from a former employee from 2 years ago referencing this post).

However, we still we have some dangling threads as evidenced by there still being outdated language in our repository. This RFC hopes to introduce some behaviour changes that will make it so we don't have the same conversation in a year.

Master --> Main
In investigating the problem, engineering directors noticed that maybe people didn't feel comfortable renaming repositories, so this pull request updates the playbook for that.

Allow/Deny
We also noticed that while we are migrating to allow/deny, some of our included dependencies use outdated language. We should not let it stop us that the outdated language comes from outside our organisation if we want to lead with openness, and follow through on myriad commitments to diversity as claimed here. To that end we should make sure in a pull request to notice use of calling a "blacklist/whitelist." If it's inside our org's repositories, we should either fix within that PR or open an issue if it is large surgery. If it's outside our organization, we should update the dependency if the other org has already modernised. If the org has not yet deprecated outdated language, we should either open a pull request or file an issue depending on the size of the problem.

How is this RFC resolved?

  1. Agree to spend time updating repositories, dependencies, and linters. Engineering management suggests mobbing on it this Friday December 10. To follow along:
  1. Reminder set on this RFC to check in 6 months for progress by searching our org for outdated language.
@artsy-peril artsy-peril bot added the RFC label Nov 22, 2021
@lidimayra
Copy link

lidimayra commented Nov 23, 2021

I completely agree. I also wonder if the RFC resolution steps could include having our teams upgrade rubocop in the owned projects.

Rubocop includes the InclusiveLanguage rule (link) since v1.23.0.

And having our linter updated sounds healthy to our codebase in many other aspects as well.

@kathytafel
Copy link
Contributor Author

@lidimayra I added the note for linter. Is that sufficient, since linters will be different per repository/language?

@lidimayra
Copy link

Yes, I think so.

I mentioned rubocop, because this is the linter that we already use in our ruby-based projects and it would be a matter of updating what we already have. I don't know about the options for other languages, but it would surely be a nice opportunity to find that out for each repo.

@anandaroop
Copy link
Member

Thanks, good to have this made explicit.

I think many of us have implicitly followed this since those Slack conversations (the precipitating example predates that), but there's also been a lot of new faces since then, and having an RFC to point to makes this clearer and more actionable.

Glad to learn about that new Rubocop setting (I hesitated to call it a "cop" because, well, you know 😅)

Seems that ESLint also has a similar plugin.

I think I'd vote to have these nudges via linting than via a checklist because:

  • Not every repo uses a PR template
  • I suspect these are very infrequent interventions (the master/main thing would only be once per project?) so I'd hesitate to add more process to every PR in order to socialize this
  • A linter error is a stronger nudge than a checkbox which could easily be absentmindedly or deliberately ticked without addressing the underlying issue

Given that our two most common languages have the option to automate this, might we consider that instead of the checkbox?

@lidimayra
Copy link

Glad to learn about that new Rubocop setting (I hesitated to call it a "cop" because, well, you know 😅)

very good point, I also edited my message to remove this from the original sentence, thanks for raising it 🙌

And I'm also in favor of setting the linters over the checkboxes option

@kathytafel
Copy link
Contributor Author

@anandaroop exactly my thought on the longevity of the conversation. Most everyone in the Berlin office was hired after the conversation, for instance, and also might not have as much context for what is more an American problem, so it's good to educated every once in a while.

What about a short-lived checkbox until the baselines per repository are done. That is, main --> master migrated and linter updated with these language rules? My thought had been to not make extra work out of the normal product time. Outside of a checkbox, how would you get people to add the linter rules (also for Eigen!) and the repository name migration? We've already made it so new repositories will be called main. I agree that once those tasks are done the rest can be left to linting.

@mzikherman
Copy link
Contributor

I'm in favor of this but I would disagree with the need for any checkbox or pull request template.

What about a short-lived checkbox until the baselines per repository are done. That is, main --> master migrated and linter updated with these language rules? My thought had been to not make extra work out of the normal product time.

If we want to switch main -> master, we need to afford time to do that or live with the time it will take to do that (as people/teams find appropriate, 'maintenance mondays', 'future fridays', regularly scheduled product work as part of 20% bug fixes, motivated parties championing or working on it it, etc.). I'm not sure why a checkbox on every PR would need to happen in the meantime. Same goes for any updated linter rules. Strongly in favor of automation to help identify or flag problematic language, but not necessarily in favor of PR template checkboxes otherwise.

anandaroop added a commit to artsy/rosalind that referenced this issue Dec 1, 2021
Our RFC suggests 'denylist' as an alternative to 'blacklist', so we add
that along with the built-in suggestion of 'blocklist'

artsy/README#427
anandaroop added a commit to artsy/rosalind that referenced this issue Dec 1, 2021
Our RFC suggests 'denylist' as an alternative to 'blacklist', so we add
that along with the built-in suggestion of 'blocklist'

artsy/README#427
@anandaroop
Copy link
Member

Yeah I'm also not confident that a checkbox would have the intended effect, without a lot more prescriptiveness about the who/when/what of the action that it's supposed to induce 🤷🏽

But I do feel like the linting approach is super-clear. Who = you when you get the message. When = when you commit. What = whatever the linter is noodging you about.

I thought it could be helpful to see an example of the work involved in adding that linting to a project. So I used Rosalind as a guinea pig and opened artsy/rosalind#586 to demonstrate this approach for both Ruby and Javascript

My takeaways:

  • adding JS linting very easy
  • adding Ruby linting potentially a little more involved but still manageable
  • this approach might still be prone to inconsistency between projects (i.e. configured custom word lists)
  • having a clear exemplar PR like this might help with some of the above

@kathytafel
Copy link
Contributor Author

I've updated to remove the PR checkbox. Agree that it just needs time to be done. Engineering management suggests we use this Future Friday to address.

@pvinis
Copy link
Contributor

pvinis commented Dec 6, 2021

kinda off topic, but there is this on rubocop, rubocop/rubocop#8091 and a blog post https://metaredux.com/posts/2020/06/08/the-rubocop-name-drama-redux.html.

dblandin added a commit to artsy/kaws that referenced this issue Dec 10, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/kaws that referenced this issue Dec 10, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/horizon that referenced this issue Dec 10, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/horizon that referenced this issue Dec 10, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/frequency that referenced this issue Dec 10, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/frequency that referenced this issue Dec 10, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/aprd that referenced this issue Dec 10, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/aprd that referenced this issue Dec 13, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/update-repo that referenced this issue Dec 13, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/update-repo that referenced this issue Dec 13, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
@kathytafel
Copy link
Contributor Author

kathytafel commented Dec 13, 2021

Resolution

We decided to do it.

Level of Support

3: Majority acceptance, with conflicting feedback only about how to get things done.

Next Steps

We implemented 85% of it. Will monitor in 6 months to see what's left

Exceptions

Eidolon and Energy have extra things to fix in order to deploy.

dblandin added a commit to artsy/renovate-config that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/folio.artsy.net that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/gris that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/renovate-config that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/folio.artsy.net that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/gris that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/estella that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/hokusai that referenced this issue Dec 15, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
dblandin added a commit to artsy/hokusai that referenced this issue Dec 16, 2021
Favor inclusive language in the codebase per this [RFC][].

[RFC]: artsy/README#427
@lidimayra
Copy link

Adding a note regarding something that happened today and made me think about this RFC again.

In many scenarios during the FF mob, we manually replaced whitelist/blacklist terms by doing a "search & replace". This method prevents us from finding possible inflections of these terms.

As an example, today the RuboCop InclusiveLanguage rule was enabled on volt. By doing this, a scenario where the manual searching wouldn't suffice our needs was detected: usage of terms in the past tense (whitelisted).

I would be happy to work on subsequent FFs to attack this part of the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants