-
Notifications
You must be signed in to change notification settings - Fork 6.8k
build: simplify karma configuration #2823
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
* No longer use ts-node to build the Karma configuration (just to be able to import the browser-providers) * Transformed the browser-providers into a JS file (with only the most necessary exports) * Separates the list of possible external browsers into an extra file (as in Material 1) This change just removes unnecessary bloat from our build process and also makes running karma a bit faster, because we don't need to build TS on the fly. Also it just makes the `browser-providers`, `karma.conf.js` and `remote_browsers` very clear.
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.
LGTM
'Android5': { unitTest: {target: 'SL', required: false }}, | ||
'Safari7': { unitTest: {target: null, required: false }}, | ||
'Safari8': { unitTest: {target: 'BS', required: false }}, | ||
'Safari9': { unitTest: {target: 'BS', required: false }}, |
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.
Idle thought: looks like Browserstack still doesn't support Safari 10, but SauceLabs does. I wonder if we should try adding that (in a follow-up PR)
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 idea. I will take a look at this once this PR is merged.
* This adds Safari v10 (from Saucelabs) to the browser-providers and marks it as an optional run in the CI. As requested in angular#2823
* This adds Safari v10 (from Saucelabs) to the browser-providers and marks it as an optional run in the CI. As requested in #2823
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. |
This change just removes unnecessary bloat from our build process and also makes running karma a bit faster, because we don't need to build TS on the fly.
Also it just makes the
browser-providers
,karma.conf.js
andremote_browsers
very clear.@jelbourn I noticed this while trying to integrate coverage. And I also planned to remove unnecessary bloat of our build process (right now it's too confusing & big)