-
Notifications
You must be signed in to change notification settings - Fork 69
Parameterize URLs in API, update config, remove not needed standalone… #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
Conversation
README.md
Outdated
### **Environment Setup** | ||
|
||
First, you'll need to create and configure a deployment config file: `/infra/config/standalone.ts`. You can see `example.ts` in the same directory for a sample of what the end result should look like. Then, proceed with the deployment steps below. | ||
1. You'll need to create and configure a deployment config file: `/infra/config/prod.ts`. You can see `example.ts` in the same directory for a sample of what the end result should look like. Optionally, you can setup consig files for `staging` and `sandbox` deployments. Then, proceed with the deployment steps below. |
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.
Typo (consig
> config
) + suggestion for this:
Optionally, you can setup consig files for
staging
andsandbox
deployments.
Optionally, you can setup config files for
stagingand
sandbox deployments, based on your environment needs.
static getConnectWidgetUrl(): string { | ||
return getEnvVarOrFail("CONNECT_WIDGET_URL"); |
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.
🤩
@@ -2,14 +2,21 @@ import { getApiToken } from "./api"; | |||
import { NavigateFunction } from "react-router-dom"; | |||
import Constants from "./constants"; | |||
|
|||
function buildEnvParam(envParam: string) { | |||
return `&${envParam}=true`; |
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.
maybe leave &
out of this function? this way, if needed, we can position the result of this as the first param
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.
Good point!
@@ -32,5 +32,6 @@ export interface EnvConfig { | |||
host: string; | |||
domain: string; | |||
}; | |||
connectWidgetUrl?: string; |
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.
Could do some tweaking on the type definition so one of these is present: connectWidget
or connectWidgetUrl
… env #55