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

[MRG] Add codecov support #328

Merged
merged 1 commit into from Jan 5, 2018

Conversation

Projects
None yet
5 participants
@lesteve
Contributor

lesteve commented Dec 18, 2017

Some maintenance tasks while I was at it:

  • move part of .travis.yml to separate scripts
  • use miniconda3 rather than miniconda2 installer
  • use INSTALL_MAYAVI env variable
  • remove python nightly build (it's been failing for a while and has been ignored because it is in allowed_failure mode). I don't think it makes sense to test on python 3.7 on each PR I would say anyway. Something more useful IMHO would be to have a cron build testing sphinx master.
@codecov-io

This comment has been minimized.

codecov-io commented Dec 18, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #328   +/-   ##
=========================================
  Coverage          ?   91.17%           
=========================================
  Files             ?       26           
  Lines             ?     1723           
  Branches          ?        0           
=========================================
  Hits              ?     1571           
  Misses            ?      152           
  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 14470ed...29daecd. Read the comment docs.

@larsoner

I would use ci-helpers for the conda stuff but this is fine without it

@lesteve lesteve force-pushed the lesteve:codecov branch from 28c97a0 to ada333e Dec 18, 2017

Add codecov support
Also some maintenance tasks:
- move part of .travis.yml to separate scripts
- use miniconda3 rather than miniconda2 installer
- use INSTALL_MAYAVI env variable
- remove python nightly build

@lesteve lesteve force-pushed the lesteve:codecov branch from ada333e to 29daecd Dec 18, 2017

@lesteve

This comment has been minimized.

Contributor

lesteve commented Dec 19, 2017

@Titan-C do you want to have a quick look at this one?

@Titan-C

This comment has been minimized.

Member

Titan-C commented Dec 19, 2017

@Titan-C

This comment has been minimized.

Member

Titan-C commented Dec 22, 2017

The changes look fine. Moving from travis.yml to a script is cleaner.
I agree on not testing on the nightly version of python for the moment.
I don't get the codecov report. Pytest-cov also gives a report on coverage, that one is easy to understand.

@lesteve

This comment has been minimized.

Contributor

lesteve commented Dec 22, 2017

@@> I don't get the codecov report. Pytest-cov also gives a report on coverage, that one is easy to understand.

The codecov comment is not always useful I agree. It is more useful once this is merged into master because it then does some kind of diff between master and the PR.

I generally look at things like per-file report https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/328/tree?path=sphinx_gallery which is similar to the pytest-cov one.

There is also a very useful codecov browser extension that show you the line in the PR diff that are not covered by any tests. See https://github.com/codecov/browser-extension. Note: I don't think the codecov extension has been extended for Firefox 57 yet. I am watching codecov/browser-extension#44 and hoping this happens not too far in the future.

One last thing is that there is a codecov status that is red if your absolute coverage or the coverage of your diff is no high enough. The latter is quite useful because it tends to catch our laziness when we forget to add test for some edge cases.

@choldgraf

This comment has been minimized.

Contributor

choldgraf commented Dec 29, 2017

I'm +1 on this, but won't merge since it seems @Titan-C has some concerns

@lesteve

This comment has been minimized.

Contributor

lesteve commented Jan 5, 2018

I am going to merge this one, it's easy to revert or tweak (e.g. turning codecov comments off comes to mind) if needed.

Slightly related: I read about Optimistic Merging recently. Probably not applicable in practice but thought provoking still.

@lesteve lesteve merged commit 98d4320 into sphinx-gallery:master Jan 5, 2018

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
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

@lesteve lesteve deleted the lesteve:codecov branch Jan 5, 2018

@choldgraf

This comment has been minimized.

Contributor

choldgraf commented Jan 5, 2018

+1 on optimistic merging :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment