-
Notifications
You must be signed in to change notification settings - Fork 1
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 configuration example #26
Comments
I don't think it makes sense to use environment variables in a sample configuration file. The use of environment variables is common in the context of containers, but if you don't use container you usually don't use environment variables but you simply write the settings to the configuration file or you use your own deployment. |
If we're talking about a random OS project, sure. But this is a plugin for projects primarily (if not exclusively) worked on by ourselves, which can be assumed to get deployed in containers, plus, we don't live a culture of best practices as an org, so a template that safeguards against accidental leaks of data used to authenticate with 3rd party services can only be a good thing, IMO. |
And if we write documentation only for ourselves it will stay that way. But if we write documentation with less assumptions about the use cases, that can change |
I don't know who's responsible for this project or what its long-term goals are, so can't speak to where its priorities lie. Personally, I'd always prioritise org-first considerations because I don't believe we have the resources to properly support non-org OS work at this point (which, IMO, goes beyond open sourcing code). The project may officially be released through PyPI, but considering that for the past 4 years, no-one bothered to provide even the bare minimum of information about it in the one place people are supposed to find it, a copy-pasteable snippet in a README which makes assumptions about a user's environment should be among the least of the project's worries. I get the general argument, but would argue to revisit it whenever it becomes actually relevant and go with whatever benefits internal use until then (which... may be years). |
I think it would be a good idea to adapt the config example in the README for better reusability in real world contexts. I.e. use env variables in place of hardcoded strings for sensitive values (like keys, or in the case of Zotero e.g. the library's ID).
(Outdated) Issue which has a link to a project's modification to use the plugin with CI/CD variables for illustration: acdh-oeaw/apis-ontologies#42
Side note: it might make sense to update the formatting of the code examples in the README so they follow black's default rules (for even more convenient copy/pasting).
The text was updated successfully, but these errors were encountered: