[MRG + 1] Run codecov only once #132
Conversation
Codecov Report
@@ Coverage Diff @@
## master #132 +/- ##
=======================================
Coverage 84.47% 84.47%
=======================================
Files 12 12
Lines 1198 1198
Branches 288 288
=======================================
Hits 1012 1012
Misses 142 142
Partials 44 44Continue to review full report at Codecov.
|
|
Hey @vaibhavmule, why is this needed? Why did you move Python 3.6 in the matrix? I think the current way it is done clearly separates generation of coverage.xml and uploading it to codecov. |
|
This way it will send only send for once, which is for 3.6. |
Why is that bad? |
|
Three things,
So why not have one version to check coverage and send that to codecov. |
|
Check this image, see that it has been edited 4 times? 4 versions? Why do you want to repeat the same thing? And check this PR, no edits.
|
|
Hmm, makes sense. Though the current way doesn't look very clean. Requests does this in a nice way, check out their .travis.yml and Makefile. Can you create a Makefile and then add jobs to .travis.yml? Would also solve #114! |
|
Exactly, I can attempt for creating Makefile, and see even requests generate codecov only once. |
|
We can reuse the Requests Makefile, it should be straightforward. We need the following directives: install, test, coverage, publish and docs. For install, we can add both apt and brew commands for dependencies based on the operating system on which the Makefile is run (check out this gist). |
|
We can add flake8 later. |
|
@vinayak-mehta This is up for review. Installing dependencies via Makefile is getting failed. That is why I have moved things to before_install: 835278e |
|
@vaibhavmule Thanks! Will look into it today. |
Makefile
Outdated
| @@ -0,0 +1,16 @@ | |||
| .PHONY: docs | |||
| init: | |||
There was a problem hiding this comment.
@vaibhavmule Can you add the apt and brew install commands in here by detecting the OS? https://gist.github.com/sighingnow/deee806603ec9274fd47
There was a problem hiding this comment.
I can add that, have you seen this 835278e and seen how a build is failing if I add that to init: section: https://travis-ci.org/socialcopsdev/camelot/jobs/438470439
.travis.yml
Outdated
| - stage: coverage | ||
| python: 3.6 | ||
| script: make test && codecov --verbose |
There was a problem hiding this comment.
@vaibhavmule The requests Makefile just calls codecov at this stage? I see in the docs that files aren't preserved between stages, any idea how they get the coverage.xml to upload at this stage?
There was a problem hiding this comment.
They (Requests) don't, their coverage thing is broken.
There was a problem hiding this comment.
Oh didn't know that. Any idea why?
There was a problem hiding this comment.
I'm not sure, but as far as I know, codecov requires the tests to be run and coverage file to be generated when you want send things to codecov.
This is how we do at @MasoniteFramework https://github.com/MasoniteFramework/core/blob/develop/.travis.yml#L13
There was a problem hiding this comment.
Yeah just not a big fan of the && :) Let's keep it for now till we can think of a nicer way. Maybe adding a directive in the Makefile will reduce this to a single command.
There was a problem hiding this comment.
@vinayak-mehta if not &&, does this make sense?
script:
- make test
- codecov --verbose
vinayak-mehta
force-pushed
the
patch-1
branch
from
ca09091
to
86e18f7
Compare
vinayak-mehta
changed the title
|
Fixed it! Also reworded the commit messages so that they start with a capital letter, tip for next time! @vaibhavmule https://camelot-py.readthedocs.io/en/master/dev/contributing.html#work-on-your-pull-request |
|
Thanks for the codecov catch @vaibhavmule! |
|
Now these |
|
Haha, not familiar with that emoji. |
Closes: #114