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

Suggesting using Secrets instead #2

Merged
merged 2 commits into from
Jun 15, 2023
Merged

Suggesting using Secrets instead #2

merged 2 commits into from
Jun 15, 2023

Conversation

benguild
Copy link
Contributor

#1

@benguild benguild mentioned this pull request Jun 15, 2023
@Sh4yy
Copy link
Owner

Sh4yy commented Jun 15, 2023

Thanks for looking into this! I find setting up env variables much easier and straightforward, specifically when doing local development and deploying without using Wranger. I'd say a better solution would be supporting both but giving a higher priority to secrets so it is used if set. Otherwise, we default back to env variables.

Happy to implement it tomorrow morning!

@benguild
Copy link
Contributor Author

It's just a README change. Deploying a secret with the same name works the same way but doesn't store it in the file, which risks it being accidentally committed to the repo.

@Sh4yy
Copy link
Owner

Sh4yy commented Jun 15, 2023

I apologize! You are right!

Two things before I merge,

  1. We still need [env.production] in the toml file. I currently get this warning:
    - No environment found in configuration with name "production".
      Before using `--env=production` there should be an equivalent environment section in
  the configuration.
  
      Consider adding an environment configuration section to the wrangler.toml file:
      ```
      [env.production]
      ```
  1. Also, I think this would be a better description for the third step:

Use the command npx wranler secret put --env production TOKEN to create a secure token. You are then prompted to enter a random secret value. This will be used to authenticate your requests. You can also set this value directly in your Cloudflare dashboard.

@benguild
Copy link
Contributor Author

I'll tweak it right now, hang on

@benguild
Copy link
Contributor Author

pushed 26aa4bc

@Sh4yy Sh4yy merged commit daa1faf into Sh4yy:main Jun 15, 2023
@Sh4yy
Copy link
Owner

Sh4yy commented Jun 15, 2023

Merged! Thank you.

shilingihy added a commit to shilingihy/cloudflare-email that referenced this pull request Aug 10, 2024
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.

2 participants