Conversation
brian-smith-tcril
left a comment
brian-smith-tcril
left a comment
There was a problem hiding this comment.
This is mostly identical to what landed in openedx/frontend-base#151
I left comments on the small parts I changed
I verified this works against the frontend-base branch of frontend-app-authn here: brian-smith-tcril/frontend-app-authn@e45c637
| export const FRONTEND_BASE_DIR = process.env.FRONTEND_BASE_DIR | ||
| ? path.resolve(process.env.FRONTEND_BASE_DIR) | ||
| : path.resolve('../frontend-base'); |
There was a problem hiding this comment.
This has changed from the version in test-site which was hardcoded to point to frontend-base in its parent directory.
| if (!process.env.PORT) { | ||
| throw new Error( | ||
| '[frontend-dev-utils] PORT is required. Example: PORT=1234 devutils-dev-with-autoinstall' | ||
| ); | ||
| } | ||
| export const PORT = Number(process.env.PORT); |
There was a problem hiding this comment.
This was hardcoded in the test-site version, now it needs to be set explicitly.
| "license": "AGPL-3.0", | ||
| "author": "Open edX Community", | ||
| "bin": { | ||
| "devutils-dev-with-autoinstall": "./tools/autoinstall/dev-with-autoinstall.mjs" |
There was a problem hiding this comment.
I don't love this name. If we want to get fancy with it I could move this away from directly providing the script in bin. If we had a cli.mjs then apps could have frontend-dev-utils dev-with-autoinstall instead.
There was a problem hiding this comment.
I think it's fine as-is. It's going to be abstracted away by an npm run invocation, anyway.
| "license": "AGPL-3.0", | ||
| "author": "Open edX Community", | ||
| "bin": { | ||
| "devutils-dev-with-autoinstall": "./tools/autoinstall/dev-with-autoinstall.mjs" |
There was a problem hiding this comment.
I think it's fine as-is. It's going to be abstracted away by an npm run invocation, anyway.
2 participants
No description provided.