Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Move global to /src and make CSP compatible #80

Merged
merged 2 commits into from
Jun 28, 2017
Merged

Move global to /src and make CSP compatible #80

merged 2 commits into from
Jun 28, 2017

Conversation

kitsonk
Copy link
Member

@kitsonk kitsonk commented Mar 8, 2017

Type: bug / feature

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

This adopts a CSP compliant global detection and migrates it from src/support/global.ts to src/global.ts because it is now aligning to the ES proposal for global.

Resolves #61

@agubler
Copy link
Member

agubler commented Mar 8, 2017

@kitsonk is this used by any downstream modules?

@codecov-io
Copy link

codecov-io commented Mar 8, 2017

Codecov Report

Merging #80 into master will decrease coverage by 0.22%.
The diff coverage is 86.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
- Coverage   97.13%   96.91%   -0.23%     
==========================================
  Files          30       30              
  Lines        1291     1295       +4     
  Branches      250      252       +2     
==========================================
+ Hits         1254     1255       +1     
- Misses         21       23       +2     
- Partials       16       17       +1
Impacted Files Coverage Δ
src/Set.ts 100% <100%> (ø) ⬆️
src/native/Symbol.ts 77.77% <100%> (ø) ⬆️
src/native/number.ts 100% <100%> (ø) ⬆️
src/support/queue.ts 92.47% <100%> (ø) ⬆️
src/Observable.ts 100% <100%> (ø) ⬆️
src/Promise.ts 98.18% <100%> (ø) ⬆️
src/number.ts 100% <100%> (ø) ⬆️
src/Symbol.ts 86.56% <100%> (ø) ⬆️
src/native/Promise.ts 100% <100%> (ø) ⬆️
src/WeakMap.ts 100% <100%> (ø) ⬆️
... and 7 more

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 0df5c50...c277a35. Read the comment docs.

@dylans dylans added this to the 2017.03 milestone Mar 8, 2017
@agubler
Copy link
Member

agubler commented Mar 8, 2017

This is fine if there are no existing uses of support/global otherwise it will not be backwards compatible.

@kitsonk
Copy link
Member Author

kitsonk commented Mar 8, 2017

is this used by any downstream modules?

Nothing depended upon it explicitly, so this is additive. Remove of it from core thought (dojo/core#303) will cause some breakage. Maybe we should alias that for now until we can clean it up everywhere else.

@kitsonk
Copy link
Member Author

kitsonk commented Mar 8, 2017

Also, it is slightly annoying me that we aren't testing the web worker code path too... we should really do that.

@dylans
Copy link
Member

dylans commented May 19, 2017

Same note to revisit this one as well @kitsonk , thanks!

@eheasley eheasley modified the milestones: 2017.05, 2017.06 Jun 6, 2017
@kitsonk
Copy link
Member Author

kitsonk commented Jun 28, 2017

Right now it is too difficult to test the web worker path and then collect the coverage information. We don't really have a choice but to accept the untested code.

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.

5 participants