-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(ng-deploy): add option for buildTarget #2281
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(ng-deploy): add option for buildTarget #2281
Conversation
@@ -33,12 +36,15 @@ export default createBuilder<any>( | |||
context.target.project | |||
); | |||
|
|||
const buildTarget = options.buildTarget || `build:${context.target.project}:production`; |
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.
Note
I'm not sure this is the best approach. Ideally ng add @angular/fire
would automatically define project.architect.deploy.options.buildTarget
in angular.json
.
This could easily be done here schematics / ng-add.ts #L231
.
Pros
We could make buildTarget
required and make configuration more obvious for developers.
Cons
This could cause backwards compatibility issues.
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.
Agreed, it should be automatically defined on ng add
. However, backwards compatibility should be maintained ie a default fallback value.
LGTM, will bring into the next release this week. |
Co-authored-by: James Daniels <james@jamesdaniels.net>
This has been released in both |
Thank you for adding this! It would be great to get some documentation for it in https://github.com/angular/angularfire/blob/master/docs/deploy/getting-started.md |
The description mentions using |
@Splaktar feel free to open a PR with your findings to update the docs. I'll be happy to review & merge. |
This is correct - the original description has a mistake - it's Also, I'm assuming PS: I've added the ability to specify a firebase project deploy target in PR #2366 EDIT: I've committed a docs update to my last pull request describing these features/options |
Checklist
yarn install
,yarn test
run successfully? yesDescription
Add an option for buildTarget so that any builder can be scheduled.
Code sample
ng deploy --buildTarget=project:browser:staging
or