Skip to content

added codecov support#645

Closed
shubhank-saxena wants to merge 2 commits into
openwpm:masterfrom
shubhank-saxena:codecov
Closed

added codecov support#645
shubhank-saxena wants to merge 2 commits into
openwpm:masterfrom
shubhank-saxena:codecov

Conversation

@shubhank-saxena

Copy link
Copy Markdown
Contributor

This fixes #640
We still need to register the repo on codecov so that the test coverage can be picked up!
@vringar

Comment thread .travis.yml
@@ -109,3 +117,4 @@ jobs:
after_success:
- # npx nyc report --reporter=lcov | npx codecov

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this line still here, did we use to have codecov enabled before?
And why should we do it differently now than we did before?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's something I missed 😮

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I followed up using npm. Can be done this way too

@vringar

vringar commented May 12, 2020

Copy link
Copy Markdown
Contributor

Thanks for submitting this PR.
There are a couple of points that are still unclear to me only some of which can be addressed by you:

  • Who do I need to talk to, to get this enabled for us? (That's probably a task for me)
  • Shouldn't these tools be in requirements-dev.in and package.json respectively?
  • Do we want to create policies around this tool (e.g. no lowering the code coverage) or is it just informative

@shubhank-saxena

Copy link
Copy Markdown
Contributor Author
  • We can add them in the respective requirement files. But codecov only runs just on the Travis. So this way it won't clutter the packages. If you want, I can shift them.
  • Currently, this is informative only. Can be toggled to a policy if and when required (like a threshold that every PR should at least maintain 40%+ coverage and shouldn't dip it

@shubhank-saxena shubhank-saxena changed the title added codecov.yml file added codecov support May 12, 2020
@birdsarah

Copy link
Copy Markdown
Contributor

We can add them in the respective requirement files. But codecov only runs just on the Travis. So this way it won't clutter the packages. If you want, I can shift them.

I think we should include in dev requirements. It's small and it makes sure everywhere's working with the same set of dependencies which makes it easier to track down problems.

The rebase now #648 is in will probably be a bit gross. Apologies. You may just want to start a new branch. The good news is the PR will be smaller as .travis.yml shrunk considerably.

@birdsarah birdsarah marked this pull request as draft May 18, 2020 23:39
@shubhank-saxena

Copy link
Copy Markdown
Contributor Author

@birdsarah sure! Will start a new PR! #648 is literally a great thing! 🚀

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.

Addition of codecov for analyisng test coverage

3 participants