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

Removing cover-min-percentage=100 from coveralls. #940

Merged
merged 1 commit into from
Jun 23, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jun 23, 2015

Fixes #939.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 23, 2015
@tseaver
Copy link
Contributor

tseaver commented Jun 23, 2015

We dropped running tox -e cover in the "main" tests in Travis, which would block the tox -e coveralls bit (in after_success). Is a drop in coverage still going to show up as a failed check?

@dhermes
Copy link
Contributor Author

dhermes commented Jun 23, 2015

@tseaver The coverage/coveralls check will fail but the continuous-integration/travis-ci/pr check will not. Is that sufficient?

@dhermes
Copy link
Contributor Author

dhermes commented Jun 23, 2015

I can add a fake PR after this for us to test. Something silly like

if False:
    print 'I will never happen'  # Maybe though because False is just a variable

@tseaver
Copy link
Contributor

tseaver commented Jun 23, 2015

If we're sure that PRs will show up with failed checks, then go ahead. I don't really see the value-add of coveralls is over just running tox -e cover within Travis, but if we're going to use it, we should definitely let it work as expected.

@dhermes
Copy link
Contributor Author

dhermes commented Jun 23, 2015

We use coveralls for the fancy badge on the front page. cover is in the default list, so running tox locally will run tox -e cover.

I'm taking your "go ahead" as an LGTM. Happy to rollback if I misunderstood.

dhermes added a commit that referenced this pull request Jun 23, 2015
Removing cover-min-percentage=100 from coveralls.
@dhermes dhermes merged commit 8622fa0 into googleapis:master Jun 23, 2015
@dhermes dhermes deleted the fix-939 branch June 23, 2015 20:15
@dhermes dhermes mentioned this pull request Jun 23, 2015
@dhermes dhermes mentioned this pull request Jul 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants