-
Notifications
You must be signed in to change notification settings - Fork 48
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
feat: add start:with-theme and build:with-theme NPM scripts with Paragon CLI #641
base: master
Are you sure you want to change the base?
Conversation
b683cf1
to
f990ab8
Compare
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #641 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 2 2
Lines 7 7
======================================
Misses 7 7
☔ View full report in Codecov by Sentry. |
"i18n_extract": "BABEL_ENV=i18n fedx-scripts babel src --quiet > /dev/null", | ||
"lint": "fedx-scripts eslint --ext .js --ext .jsx .", | ||
"lint:fix": "fedx-scripts eslint --fix --ext .js --ext .jsx .", | ||
"snapshot": "fedx-scripts jest --updateSnapshot", | ||
"start": "fedx-scripts webpack-dev-server --progress", | ||
"start:with-theme": "paragon install-theme --brand=@edx/brand-edx.org@latest && npm start", |
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.
Can this be made generic, somehow? Otherwise, the command would warrant being called start:with-edx-theme
... and in that case, including it here would be debatable.
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.
@arbrandes Ah, yes. Let me mark this PR as a draft as it's still in flux a bit. The paragon install-theme
command does now accept a --brand
argument for more programmatic usage of the CLI tool (e.g., in build+deploy processes, if needed). But, we do not intend to include --brand
as part of the NPM scripts in MFEs.
Instead, if the --brand
argument is not passed, it prompts the user in terminal for the name/version of an NPM package to install as the @edx/brand
alias instead.
We also heard some feedback recently, where a third usage pattern for this command might be a Git-ignored config file where an engineer could specify something like the following to persist the custom @edx/brand
theme selection without requiring manual input each time.
// paragon-cli.config.js, gitignored
{
'commands': {
'install-theme': {
'brand': '@edx/brand-edx.org@latest',
}
}
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.
The latter looks like a neat way out!
The initial logic for starting a project with a theme was added in this pull request |
Integrates the base frontend-template-application MFE with the Paragon CLI command
install-theme
.Allows engineers doing local development with custom
@edx/brand
NPM packages to run the custom brand without risking modifying/committing code with the custom@edx/brand
package still installed.