-
Notifications
You must be signed in to change notification settings - Fork 12k
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(@angular-devkit/build-angular): add --reporters option to test #12404
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.
Can you add the definition also in here please:
"karma": { |
if (options.reporters) { | ||
// Split along commas to make it more natural, and remove empty strings. | ||
karmaOptions.reporters = options.reporters | ||
.reduce<string[]>((acc, curr) => acc.concat(...curr.split(/,/)), []) |
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.
Is the spread operator really needed here togather with concat
?
If you want to use the spread it should be;
[
...acc,
...curr.split(/,/),
]
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.
Removed.
It was a regression, and used by enough people on CI. No reason it should be omitted and karma.conf.js only. Fixes angular#11376
@alan-agius4 Done both comments; added to the angular.json schema and removed spread. PTAL. |
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.
Thanks for the updates. LGTM.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
It was a regression, and used by enough people on CI. No reason it should be
omitted and karma.conf.js only.
Fixes #11376