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

Example with slack_channel and slack_username in secrets.json #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jordan8037310
Copy link
Contributor

Provided an example for slack_channel and slack_username to be added to secrets.json per #99

@pantheon-mentionbot
Copy link

@jordan8037310, thanks for your PR! By analyzing the blame information on this pull request, we identified @ari-gold, @joshkoenig and @greg-1-anderson to be potential reviewers

@joshkoenig
Copy link
Member

No sure how I feel about having non-secret things in secrets.json. It's not really meant to be a config system.

Further, in my experience real world implementations often have multiple channels/etc going on, which to me is more intuitive and easier to deploy if expressed in code, versioned, and deployed per usual, rather than relying on config being deployed to all environments as well.

I put this in a line comment on #101, but maybe we can dig into the thinking behind this change here?

@jordan8037310
Copy link
Contributor Author

Responded in #101, the issue here is that secrets.json is already looking for the information there, and I was just going off the assumption that was a good idea :)

@greg-1-anderson
Copy link
Member

I originally wrote the code to use secrets.json as a place to store configuration for the same reason: since it is there and convenient. However, since this project is an "examples" project, the theory is that these are not tools that folks will track (e.g. get updates for); instead, folks will just make their own fork and customize to suit. It was therefore felt that non-secret constants can just be replaced right in the code. That's why we took out secrets-as-configuration the first time this was discussed.

If you have a single Quicksilver script that you want to use across multiple sites, you could make your own Quicksilver Scripts repository, and make configurable / re-usable scripts there. The Terminus Quicksilver plugin supports the notion of private / personal repositories.

@ccharlton
Copy link
Collaborator

Having an associated JSON config file within the Quicksilver integration [plugin] seems to be a common pattern here, so stick with that.

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

Successfully merging this pull request may close these issues.

5 participants