-
Notifications
You must be signed in to change notification settings - Fork 1
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
updates fly configs #189
updates fly configs #189
Conversation
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.
Nothing critical, I'm mostly just worried that we'll look back at this in a couple weeks and wonder what the hell we were up to.
ARG NEXT_PUBLIC_RPC_ENDPOINT | ||
ARG NEXT_PUBLIC_WALLETCONNECT_PROJECT_ID | ||
ARG NEXT_PUBLIC_APP_URL | ||
ARG API_ENDPOINT |
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 still think we should figure out how to avoid this, but its ok for now. We should document it clearly though, since it isn't obvious why this is done this way.
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 understandable needs a comment/flag
web-portal/frontend/fly.staging.toml
Outdated
NEXT_PUBLIC_WALLETCONNECT_PROJECT_ID = 'wallet-connect-project-id' | ||
NEXT_PUBLIC_RPC_ENDPOINT = 'rpc-endpoint' |
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 actually setting the environment variables to this, they get overwritten by secrets, but it isn't clear that this is a great way to document the need for the secrets. Not sure though.
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 agree but we shouldn't put our secrets in repo anyway
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.
Right, but this doesn't do anything right here, so a bit confusing.
web-portal/frontend/fly.toml
Outdated
NEXT_PUBLIC_WALLETCONNECT_PROJECT_ID = 'wallet-connect-project-id' | ||
NEXT_PUBLIC_RPC_ENDPOINT = 'rpc-endpoint' |
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.
Same complaint
node_modules