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

Update saintmode configuration language for inclusivity #217

Open
benzvan opened this issue Oct 23, 2023 · 5 comments
Open

Update saintmode configuration language for inclusivity #217

benzvan opened this issue Oct 23, 2023 · 5 comments

Comments

@benzvan
Copy link
Contributor

benzvan commented Oct 23, 2023

I was cleaning up some of the archaic language in our projects and discovered we're setting the saintmode.blacklist value on our varnish containers. This terminology should be updated to something more inclusive and more descriptive of its purpose. It would make sense to deprecate the configuration field and replace it with backend_sick_timer or similar descriptive language. Honestly the whole saint, grace, etc. metaphor could be more descriptive.

Instances in code:
https://github.com/search?q=repo%3Avarnish%2Fvarnish-modules%20blacklist&type=code

More details:
https://www.splunk.com/en_us/blog/learn/blacklist-whitelist-inclusivity.html

@gquintard
Copy link
Collaborator

I'm in favor, would you care opening a PR for it?

@benzvan
Copy link
Contributor Author

benzvan commented Oct 23, 2023

I can give it a shot. It's been a while since I've worked in C, much less varnish.

@gquintard
Copy link
Collaborator

thankfully, you shouldn't have to do C, Varnish 7.1 introduced $Alias that should do your bidding. You can see it used in vmod-cookie

That will leave the old names around for a while, but I don't want to break users without notice

@benzvan
Copy link
Contributor Author

benzvan commented Oct 24, 2023

Oh cool! It looks like I can just update the aliases and documentation and leave the code the same, so here's a draft PR doing that. Let me know if this is what you were thinking. Updating the code looks pretty straightforward but I'm not sure I can run the tests locally right now.

@gquintard
Copy link
Collaborator

cleaning, the associated PR has been merged

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

No branches or pull requests

2 participants