Skip to content
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

Update to marker-timing.js #1830

Merged
merged 4 commits into from Mar 8, 2019
Merged

Update to marker-timing.js #1830

merged 4 commits into from Mar 8, 2019

Conversation

ael-mas
Copy link
Contributor

@ael-mas ael-mas commented Mar 7, 2019

fixing #1827

@codecov
Copy link

codecov bot commented Mar 7, 2019

Codecov Report

Merging #1830 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1830      +/-   ##
==========================================
- Coverage   84.09%   84.07%   -0.02%     
==========================================
  Files         179      179              
  Lines       12435    12437       +2     
  Branches     3067     3065       -2     
==========================================
  Hits        10457    10457              
- Misses       1798     1800       +2     
  Partials      180      180
Impacted Files Coverage Δ
src/profile-logic/marker-timing.js 88.88% <0%> (-5.23%) ⬇️
src/components/js-tracer/Settings.js 100% <0%> (ø) ⬆️
src/app-logic/url-handling.js 82.24% <0%> (ø) ⬆️

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 3500ec8...20635fa. Read the comment docs.

@ael-mas
Copy link
Contributor Author

ael-mas commented Mar 7, 2019

Could someone help me understand what the codecov tests mean when they're failing? I'm looking at the details and can't seem to wrap my head around what's happening.

Copy link
Member

@canova canova left a comment

Choose a reason for hiding this comment

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

hey @ael-mas, thanks for the PR! It looks good to me.

For the codecov, it's not required to pass to merge the PR. Generally it's good to add some tests when you change something and that tool helps us to understand if we are missing to add some tests. Currently you added a new branch and that branch is not covered by test(that means while running all the tests we are not going inside that branch and we are not executing these codes). But before this patch we had no specific tests for this function as well and other paths are also not covered so it's okay to land this.

@canova canova merged commit 3bd5a98 into firefox-devtools:master Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants