Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Allowing browser flag in serve #167

Merged
merged 3 commits into from
Jun 2, 2017
Merged

Allowing browser flag in serve #167

merged 3 commits into from
Jun 2, 2017

Conversation

Blackbaud-BobbyEarl
Copy link

Tested in OSX and Windows against chrome, firefox, safari, iexplore, and edge.

A few notes:

  • edge has to be a hard-coded use-case since they only expose a protocol microsoft-edge: instead of an executable.
  • This only updates the skyux serve command. Support could also be added for the skyux test, skyux watch, and skyux e2e commands.

Fixes blackbaud/skyux2#402

@codecov-io
Copy link

codecov-io commented Jun 1, 2017

Codecov Report

Merging #167 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #167      +/-   ##
==========================================
+ Coverage   96.06%   96.09%   +0.03%     
==========================================
  Files          38       38              
  Lines         838      845       +7     
  Branches      112      113       +1     
==========================================
+ Hits          805      812       +7     
  Misses         33       33
Flag Coverage Δ
#builder 100% <100%> (ø) ⬆️
#runtime 100% <ø> (ø) ⬆️
#srcapp 58.75% <ø> (ø) ⬆️
Impacted Files Coverage Δ
config/webpack/serve.webpack.config.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5a81d0...6b0afa7. Read the comment docs.

Copy link
Member

@Blackbaud-PaulCrowder Blackbaud-PaulCrowder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest changing the expect(<string>).toContain(<url>) lines to be expect(<string>.indexOf(<url>).toBe(0) since you really want to ensure the string starts with the URL and not that it just contains it, since an errant space or other character could cause the test to pass but the code to fail.

@Blackbaud-BobbyEarl
Copy link
Author

Good suggestion @Blackbaud-PaulCrowder. Incorporated.

@Blackbaud-BobbyEarl
Copy link
Author

Just a heads up @blackbaud-johnly, this is something we'll want to document. I started to write it, but saw the critiques already flowing, so I'll just save us both time and ask that you do it. :-)

@Blackbaud-BobbyEarl Blackbaud-BobbyEarl merged commit 81b5af1 into master Jun 2, 2017
@Blackbaud-BobbyEarl Blackbaud-BobbyEarl deleted the browser-flag branch June 2, 2017 20:24
@blackbaud-johnly
Copy link
Contributor

Thanks @Blackbaud-BobbyEarl . I already had a note to ask you about this next week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants