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

[Breakpoints] setting a condition should enable a breakpoint #3754

Closed
wants to merge 3 commits into from
Closed

[Breakpoints] setting a condition should enable a breakpoint #3754

wants to merge 3 commits into from

Conversation

sagorika1996
Copy link
Contributor

Associated Issue: #3730

Summary of Changes

  • The disabled breakpoint is replaced with a conditional breakpoint.
  • Added test

Copy link
Contributor

@jasonLaster jasonLaster left a comment

Choose a reason for hiding this comment

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

Looks great to me!

Lets see if the tests pass

@codehag
Copy link
Contributor

codehag commented Aug 22, 2017

There is an eslint issue

yarn lint-js

yarn lint-js v0.24.5
$ eslint *.js "src/**/*.js" 

/home/ubuntu/debugger.html/src/actions/breakpoints.js
  294:1  error  Delete `····`  prettier/prettier

(don't worry about the warnings, we will take care of that separately!)

@jasonLaster jasonLaster changed the title Breakpoint [Breakpoints] setting a condition should enable a breakpoint Aug 23, 2017
@codecov
Copy link

codecov bot commented Aug 23, 2017

Codecov Report

Merging #3754 into master will increase coverage by 0.37%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3754      +/-   ##
==========================================
+ Coverage   56.14%   56.52%   +0.37%     
==========================================
  Files         118      118              
  Lines        4679     4639      -40     
  Branches      965      957       -8     
==========================================
- Hits         2627     2622       -5     
+ Misses       2052     2017      -35
Impacted Files Coverage Δ
src/actions/breakpoints.js 72.81% <100%> (+2.91%) ⬆️
src/actions/ast.js 76.78% <0%> (-1.79%) ⬇️
src/utils/breakpoint/index.js 78.46% <0%> (-0.33%) ⬇️
src/selectors/visibleBreakpoints.js 0% <0%> (ø) ⬆️
src/utils/parser/index.js 100% <0%> (ø) ⬆️
src/actions/breakpoints/addBreakpoint.js 100% <0%> (ø) ⬆️
assets/images/Svg.js 66.66% <0%> (ø) ⬆️
src/components/SecondaryPanes/Expressions.js 32.18% <0%> (ø) ⬆️
src/utils/prefs.js 100% <0%> (ø) ⬆️
src/reducers/expressions.js 97.95% <0%> (+0.13%) ⬆️
... and 8 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 9e7e19f...84920b6. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 23, 2017

Codecov Report

Merging #3754 into master will increase coverage by 0.37%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3754      +/-   ##
==========================================
+ Coverage   56.14%   56.52%   +0.37%     
==========================================
  Files         118      118              
  Lines        4679     4639      -40     
  Branches      965      957       -8     
==========================================
- Hits         2627     2622       -5     
+ Misses       2052     2017      -35
Impacted Files Coverage Δ
src/actions/breakpoints.js 72.81% <100%> (+2.91%) ⬆️
src/actions/ast.js 76.78% <0%> (-1.79%) ⬇️
src/utils/breakpoint/index.js 78.46% <0%> (-0.33%) ⬇️
src/selectors/visibleBreakpoints.js 0% <0%> (ø) ⬆️
assets/images/Svg.js 66.66% <0%> (ø) ⬆️
src/utils/parser/index.js 100% <0%> (ø) ⬆️
src/utils/prefs.js 100% <0%> (ø) ⬆️
src/actions/breakpoints/addBreakpoint.js 100% <0%> (ø) ⬆️
src/components/SecondaryPanes/Expressions.js 32.18% <0%> (ø) ⬆️
src/reducers/expressions.js 97.95% <0%> (+0.13%) ⬆️
... and 8 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 9e7e19f...84920b6. Read the comment docs.

@jasonLaster
Copy link
Contributor

and re-opening so i can re-run CI

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.

3 participants