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

[WIP] Enable codecov #4631

Merged
merged 6 commits into from Mar 29, 2019

Conversation

Projects
None yet
5 participants
@pentschev
Copy link
Member

commented Mar 26, 2019

Solves #2218.

cc @mrocklin @jakirkham @TomAugspurger

  • Tests added / passed
  • Passes flake8 dask

@pentschev pentschev changed the title Enable codecov [WIP] Enable codecov Mar 26, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Mar 26, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@cb58d97). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #4631   +/-   ##
=========================================
  Coverage          ?   91.02%           
=========================================
  Files             ?       92           
  Lines             ?    17165           
  Branches          ?        0           
=========================================
  Hits              ?    15624           
  Misses            ?     1541           
  Partials          ?        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 cb58d97...fa87b94. Read the comment docs.

@pentschev

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

Looks like we have something here already. We should probably tweak what options we want to enable, and maybe decide on enforcement policy. For now I suggest we don't enforce coverage maintenance (i.e., that it's never negative) but provide this as a tool for developers to check how PRs affect the project, at least until we are comfortable that options we set suit our needs.

@jakirkham

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Generally seems ok to me.

Would slightly prefer if we could turn the comments off (to cutdown on noise). Also would be good to enable the status information so we can link to the details on their website as needed.

@pentschev

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

Generally seems ok to me.

Would slightly prefer if we could turn the comments off (to cutdown on noise). Also would be good to enable the status information so we can link to the details on their website as needed.

Agreed.

pentschev added some commits Mar 26, 2019

@pentschev

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

I think to get codecov statuses we need to install its Codecov Github app. Could someone with permissions do that?

@mrocklin

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

Done, I think

@pentschev

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

We have status now, thanks @mrocklin for installing the app. I've attempted to add coverage for each submodule, but they all seem to redirect to the same place, I'm not sure if that's because there's no report (we have to merge to get it first) or something wrong with the configuration.

Either way, would you say we should have codecov status for each submodule as different projects or just a single one? I'm personally fine with either.

@jakirkham

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Thanks Matt!

No strong feelings. We might be ok with just one report near term. However if that's difficult to do, wouldn't worry about it.

@pentschev

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

@pentschev

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Ok, we now have the status for a single project setup. Unless someone already knows about other options that we may want, I would suggest we merge this PR and start looking at codecov comparisons after this to improve it further, since I don't think we can see the comparisons before it gets merged and creates a baseline report.

@mrocklin

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

OK to merge?

@jrbourbeau

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

+1 from me

@mrocklin mrocklin merged commit c7e35aa into dask:master Mar 29, 2019

4 checks passed

codecov/patch Coverage not affected.
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pentschev pentschev deleted the pentschev:enable-codecov branch Apr 17, 2019

asmith26 added a commit to asmith26/dask that referenced this pull request Apr 22, 2019

Enable codecov (dask#4631)
* Add default .coverage.yml, but comments off and range 90...100

jorge-pessoa pushed a commit to jorge-pessoa/dask that referenced this pull request May 14, 2019

Enable codecov (dask#4631)
* Add default .coverage.yml, but comments off and range 90...100
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.