Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Get environment#3115

Merged
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
jasonLaster:get-environment
Jun 7, 2017
Merged

Get environment#3115
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
jasonLaster:get-environment

Conversation

@jasonLaster

@jasonLaster jasonLaster commented Jun 6, 2017

Copy link
Copy Markdown
Contributor

Associated Issue: #3072

Summary of Changes

This adds support for the new on demand frame scopes. It's important though that we don't remove the old system until the server has switched over.

  • adds a firefox command for getting an environment
  • adds a redux action and reducer for saving frame scopes
  • updates the scopes component and util to support frame scopes

Test Plan

  • Add some action unit tests

Next Steps

  • Add the debugger client method
  • Add the server endpoint

@codehag codehag left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

some nits, but looks good!

Comment thread src/reducers/pause.js Outdated
return {
...state,
frameScopes: { ...state.frameScopes, [selectedFrameId]: scopes },
...{ selectedFrameId }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

probably this can just be selectedFrameId

const baz = 3;
const a = 4, b = 5;
const a = 4,
b = 5;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const b = 5; would be easier to read

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

agreed, it's prettier tho

@codecov

codecov Bot commented Jun 7, 2017

Copy link
Copy Markdown

Codecov Report

Merging #3115 into master will decrease coverage by 1.57%.
The diff coverage is 17.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3115      +/-   ##
==========================================
- Coverage   66.86%   65.28%   -1.58%     
==========================================
  Files          76       76              
  Lines        2698     2708      +10     
  Branches      544      546       +2     
==========================================
- Hits         1804     1768      -36     
- Misses        894      940      +46
Impacted Files Coverage Δ
src/selectors.js 100% <ø> (ø) ⬆️
src/utils/frame.js 85.57% <ø> (ø) ⬆️
src/client/firefox/commands.js 18.81% <0%> (-0.78%) ⬇️
src/reducers/pause.js 19.48% <0%> (-11.08%) ⬇️
src/utils/scopes.js 86.15% <100%> (ø) ⬆️
src/actions/pause.js 11.53% <33.33%> (-6.47%) ⬇️
src/client/firefox/create.js 18.18% <50%> (-6.82%) ⬇️
src/actions/ast.js 37.5% <0%> (-35%) ⬇️
src/reducers/ast.js 65.71% <0%> (-22.86%) ⬇️
... and 2 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 2a3e034...c10d06d. Read the comment docs.

@jasonLaster jasonLaster merged commit e7bad50 into firefox-devtools:master Jun 7, 2017
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.

2 participants