Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

CI for coverage report setup #219

Merged
merged 7 commits into from
Sep 7, 2018
Merged

CI for coverage report setup #219

merged 7 commits into from
Sep 7, 2018

Conversation

AWolf81
Copy link
Collaborator

@AWolf81 AWolf81 commented Sep 6, 2018

Related Issue:
#214

Summary:
First coverage report for the branch is available here.

Codecov report can be found here.

Danger.js: I think we can add it later. I've looked a bit into it but I stopped at creation of a guppy-bot Github account. It would require to create a new account with an e-mail address and that feels strange.

OK, I think this is ready now. The codecov comment is probably missing in this PR because codecov is expecting the codecov.yml in master.

Info from the docs:
The following command helps to validate the config file: curl --data-binary @codecov.yml https://codecov.io/validate

@Haroenv
Copy link
Collaborator

Haroenv commented Sep 6, 2018

jest already uses Istanbul, I don't think it's necessary if you run jest with --coverage

I might be mistaken, but see: istanbuljs/nyc#524

package.json Outdated Show resolved Hide resolved
@AWolf81 AWolf81 changed the title [WIP] CI for coverage report setup CI for coverage report setup Sep 6, 2018
Copy link
Owner

@joshwcomeau joshwcomeau left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks so much for tackling this :)

behavior: default
require_changes: true # if true: only post the comment if coverage changes
require_base: no # [yes :: must have a base report to post]
require_head: yes # [yes :: must have a head report to post]
Copy link
Owner

Choose a reason for hiding this comment

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

If I understand this correctly, it will show a comment if:

  • The branch being merged in has changed the coverage in some way
  • The branch being merged in has a codecov report available
  • Whether or not the merge-target (usually master) has a codecov report

I think require_changes should be false - I like the idea of having a little indication of "Coverage was not affected by this PR. I don't have strong feelings though :) happy to leave it as-is for now and update later if we feel the need

@joshwcomeau joshwcomeau merged commit 4a86e73 into master Sep 7, 2018
@joshwcomeau joshwcomeau deleted the ci-coverage-setup branch September 7, 2018 11:08
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