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

src/broker/broker.c: Fix typo flux_repond -> flux_respond #851

Merged
merged 1 commit into from Oct 13, 2016

Conversation

Projects
None yet
5 participants
@chu11
Copy link
Contributor

chu11 commented Oct 13, 2016

TSIA :-)

@garlick garlick added the review label Oct 13, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 13, 2016

Coverage Status

Coverage decreased (-0.008%) to 75.135% when pulling ee40d3e on chu11:fixfluxrespondtypos into 286b6e7 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 13, 2016

Current coverage is 71.50% (diff: 0.00%)

Merging #851 into master will decrease coverage by 0.01%

@@             master       #851   diff @@
==========================================
  Files           157        157          
  Lines         26688      26688          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          19087      19084     -3   
- Misses         7601       7604     +3   
  Partials          0          0          
Diff Coverage File Path
0% src/broker/broker.c

Powered by Codecov. Last update 286b6e7...ee40d3e

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 13, 2016

Merging, though Al asked why this failed codecov and I didn't really have a good answer.

I get that the diff had no test hits and that this is a bad thing. In this case the error paths have little hope of gaining coverage and so we can just ignore.

The project coverage change should have been zero, so do we need to adjust codecov project thresholds to allow for some noise?

@garlick garlick merged commit c65b665 into flux-framework:master Oct 13, 2016

2 of 4 checks passed

codecov/patch 0.00% of diff hit (target 71.51%)
Details
codecov/project 71.50% (-0.02%) compared to 286b6e7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.008%) to 75.135%
Details

@garlick garlick removed the review label Oct 13, 2016

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Oct 13, 2016

Merging, though Al asked why this failed codecov and I didn't really have a good answer.

Codecov creates two cases that are checked for pass/fail

  1. codecov/project: overall project coverage does not drop by more than a configurable percent
  2. codecov/patch: the diff between master and the PR branch has a coverage that at least matches the current total project coverage

This PR failed the second case, but I kind of take the code coverage checks as advisory, because as you said, though it would be nice, it is not practical to test most error conditions. We could adjust codecov and lower the bar for the codecov/patch check, but it isn't like you can't merge the PR once you've evaluated why the PR has low coverage. (If you go to the codcov.io site for the pull request, it will show you exactly which lines in the diff are missing coverage)

I like that a red flag is raised when a PR doesn't have at least ~70% coverage (or whatever we're currently at)

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 13, 2016

I like that a red flag is raised when a PR doesn't have at least ~70% coverage (or whatever we're currently at)

I'm good with that, and thanks for repeating this explanation.

@garlick garlick referenced this pull request Oct 26, 2016

Closed

0.5.0 release notes #879

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.