-
Notifications
You must be signed in to change notification settings - Fork 10
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
v0.4.0 – rewrite: use Config and Secret Stores, enable local development #39
Conversation
9bf83d7
to
b7b28aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is well written and easy to understand.
I have a couple of questions/comments about it, but in general I think it looks good!
My main complaint about this PR is that you called it a "refactor"; to me what you made here counts more as a next version or a rewrite. =)
refactor: To rewrite existing source code in order to improve its readability, reusability or structure without affecting its meaning or behaviour.
src/cookies.rs
Outdated
const COOKIE_PREFIX: &str = "__Secure-"; | ||
|
||
// Change to "__Secure-" for production, and "local-" for local development. | ||
const COOKIE_PREFIX: &str = "local-"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if you can do something like you do with the redirect_uri (check fastly service version) so that you don't have to modify the code before deploying prod?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion, thank you!
|
||
### Deploy the Fastly service and get a domain | ||
### 3. Deploy the Fastly service and get a domain | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to mention that the COOKIE_PREFIX
needs to be changed between local and production. Or, modify the code that uses the cookie prefix to automatically use the right prefix, see my comment in cookies.rs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've modified the code so that it automatically uses the right prefix :)
Co-authored-by: Katsuyuki Omuro <[email protected]>
Co-authored-by: Katsuyuki Omuro <[email protected]>
Starter kit rewrite
This PR:
fastly.toml
setup configuration in order to remove the need forsetup.sh
.fastly.toml
local server configuration in order to enable out-of-the-box local development, pre-configured for Google OAuth clients.toml
; supersedes Bump toml from 0.5.11 to 0.8.13 #30.