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

[MRG + 1] codecov: disable comments #8502

Merged
merged 1 commit into from
Mar 4, 2017
Merged

[MRG + 1] codecov: disable comments #8502

merged 1 commit into from
Mar 4, 2017

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Mar 3, 2017

This should make some people more happy

@aabadie aabadie mentioned this pull request Mar 3, 2017
Copy link
Contributor

@nelson-liu nelson-liu left a comment

Choose a reason for hiding this comment

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

that was fast 👍

@codecov
Copy link

codecov bot commented Mar 3, 2017

Codecov Report

Merging #8502 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8502   +/-   ##
=======================================
  Coverage   95.48%   95.48%           
=======================================
  Files         342      342           
  Lines       60913    60913           
=======================================
  Hits        58160    58160           
  Misses       2753     2753

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 8c7ab17...24afc5f. Read the comment docs.

@amueller
Copy link
Member

amueller commented Mar 4, 2017

thanks :)

@amueller
Copy link
Member

amueller commented Mar 4, 2017

LGTM

@amueller amueller changed the title codecov: disable comments [MRG + 1] codecov: disable comments Mar 4, 2017
@NelleV NelleV merged commit 75c892c into scikit-learn:master Mar 4, 2017
@NelleV
Copy link
Member

NelleV commented Mar 4, 2017

Thank you! This also makes me happy :)

@amueller
Copy link
Member

amueller commented Mar 4, 2017

didn't work :(

@amueller
Copy link
Member

amueller commented Mar 4, 2017

should have been "comment". Fixing in master...

@amueller
Copy link
Member

amueller commented Mar 4, 2017

should have been

comment: off

good thing we're reviewing so well.

@lesteve
Copy link
Member

lesteve commented Mar 6, 2017

For the record I think comments were useful and the fact that you only get one email per PR rather than one per-commit was a good trade-off as far as email noise is concerned.

Having said that I agree that there is a bit of noise in the numbers that codecov reports (which we should try to understand at some point) and that the one comment per PR does not always work for some unknown reason (I thought it was due to rebase and force-push but this doesn't seem like the culprit in some small tests I ran).

@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
herilalaina pushed a commit to herilalaina/scikit-learn that referenced this pull request Mar 26, 2017
massich pushed a commit to massich/scikit-learn that referenced this pull request Apr 26, 2017
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
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.

None yet

5 participants