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 configuration example #26

Open
koeaw opened this issue Apr 12, 2023 · 4 comments
Open

Update configuration example #26

koeaw opened this issue Apr 12, 2023 · 4 comments
Labels
Documentation Issue/pr relates to documentation usability The issue /pr relates to a usability enhancement or bug

Comments

@koeaw
Copy link

koeaw commented Apr 12, 2023

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).

@koeaw koeaw added usability The issue /pr relates to a usability enhancement or bug Documentation Issue/pr relates to documentation labels Apr 12, 2023
@b1rger
Copy link
Contributor

b1rger commented Apr 12, 2023

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.

@koeaw
Copy link
Author

koeaw commented Apr 12, 2023

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.

@b1rger
Copy link
Contributor

b1rger commented Apr 13, 2023

But this is a plugin for projects primarily (if not exclusively) worked on by ourselves,

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

@koeaw
Copy link
Author

koeaw commented Apr 17, 2023

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issue/pr relates to documentation usability The issue /pr relates to a usability enhancement or bug
Projects
None yet
Development

No branches or pull requests

2 participants