-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Cloudflare] Remove netlify #2669
Conversation
❌ Deploy Preview for decent-interface-dev failed. Why did it fail? →
|
46bb520
to
0cea7fc
Compare
e917cb7
to
ad6bf5b
Compare
Deploying decent-interface with
|
Latest commit: |
8763c18
|
Status: | ✅ Deploy successful! |
Preview URL: | https://3e806634.decent-interface.pages.dev |
Branch Preview URL: | https://cf-remove-netlify.decent-interface.pages.dev |
0cea7fc
to
b5e6d69
Compare
6d789a4
to
47e53cd
Compare
b5e6d69
to
49e2afb
Compare
92dfb9c
to
3f82e36
Compare
3f82e36
to
2188358
Compare
2188358
to
91a63ec
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.
Yeah lezzgo 🤘🏼
Couple minor suggestions for cleaning up env file - but not mandatory within a scope of this PR for sure
.env
Outdated
# IPFS pinning | ||
VITE_APP_INFURA_IPFS_API_KEY="" | ||
# IPFS pinning | ||
VITE_APP_INFURA_IPFS_API_SECRET="" | ||
|
||
# App name displayed as Title | ||
VITE_APP_NAME="Decent" |
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.
Why do we need this one? Don't feel like it's reasonable to have it in env variable
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'd say we need it because it's a global level configuration string that is used in multiple places throughout the app.
If not stored as an Env Var, what would we do instead?
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.
Just a constant somewhere
My point is that it is not environment-dependant - app name is always Decent for every environment
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.
Yeah so I hear you, but I would argue to keep it here because:
- we use it multiple times in the
index.html
file, and in there we do have access to environment variables, but we don't have access to any javascript code - also, by setting
VITE_APP_NAME="Decent"
in the.env
file, the build system will pick up"Decent"
if it's not overridden in a real environment variable
This PR finally removes Netlify from the codebase.
My plan is to merge this (assuming approvals) on Sunday Night January 19th and shut down the Netlify sites at that time.