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

Coveralls integration #4009

Merged
merged 3 commits into from
May 18, 2020
Merged

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented May 16, 2020

Q A
Type improvement
BC Break no

Todo

  • Travis
  • Remove Scrutinizer
  • Update badges in the README

@greg0ire greg0ire force-pushed the coverage-with-coveralls branch 5 times, most recently from 3d6d960 to d21f291 Compare May 16, 2020 16:18
@greg0ire greg0ire force-pushed the coverage-with-coveralls branch from d21f291 to 19a9e64 Compare May 16, 2020 16:28
@greg0ire greg0ire added the CI label May 16, 2020
@greg0ire greg0ire force-pushed the coverage-with-coveralls branch 3 times, most recently from bb55360 to 4763a18 Compare May 16, 2020 18:36
@greg0ire greg0ire force-pushed the coverage-with-coveralls branch from 0d04752 to 4763a18 Compare May 17, 2020 20:52
@greg0ire greg0ire force-pushed the coverage-with-coveralls branch from 4763a18 to e02d2cf Compare May 18, 2020 07:06
@greg0ire greg0ire marked this pull request as ready for review May 18, 2020 07:12
@greg0ire greg0ire requested a review from morozov May 18, 2020 07:12
@greg0ire
Copy link
Member Author

One thing left to figure out: how do we get the coveralls bar in the Github UI?

@morozov
Copy link
Member

morozov commented May 18, 2020

One thing left to figure out: how do we get the coveralls bar in the Github UI?

You mean the badge in README? You already replaced it.

@greg0ire
Copy link
Member Author

greg0ire commented May 18, 2020

One thing left to figure out: how do we get the coveralls bar in the Github UI?

No I mean this:

Screenshot_2020-05-18 Symfony 5 0 compatibility by mathieupetrini · Pull Request #6093 · sonata-project SonataAdminBundle

Screenshot from Sonata BTW, I'm trying to understand how this works.

@greg0ire greg0ire merged commit 6aa9325 into doctrine:2.10.x May 18, 2020
@greg0ire greg0ire deleted the coverage-with-coveralls branch May 18, 2020 17:13
@morozov
Copy link
Member

morozov commented May 18, 2020

Honestly, I think it's a very annoying and useless thing. Often, if you remove fully-covered code (e.g. by replacing 10 lines with 3 lines that do the same), your average coverage drops. And this is very confusing/demotivating.

@greg0ire
Copy link
Member Author

I see we can set a theshold in the setting to avoid that kind of disheartening failure. That way we catch the big variations.

@morozov
Copy link
Member

morozov commented May 18, 2020

I'd rather like to have diff coverage to be automatically reported: the percentage of lines affected by the diff that are covered with tests. Not sure if it's supported.

@greg0ire
Copy link
Member Author

Ah it's probably just a matter of clicking the green buttons here: https://github.com/marketplace/coveralls

I don't think what you want is supported, so let's just leave things as is.

@morozov
Copy link
Member

morozov commented May 23, 2020

@greg0ire after having filed a couple of PRs and not being able to see the code coverage, I think we should try to enable it. I do not want the coverage delta to be automatically reported on the PR but I want to be able to see it if needed. Do you think we could enable it?

@greg0ire
Copy link
Member Author

greg0ire commented May 23, 2020

Sure, let's try.

EDIT: I think that app is in fact already enabled for us, so there must be some other issue.

@greg0ire
Copy link
Member Author

@morozov the coverage seems to be accessible, in fact: https://coveralls.io/jobs/62814748
I think it's just an issue with the UI. I found this link in the output of the Travis job that does coverage.

@greg0ire
Copy link
Member Author

Found the URL for all the DBAL builds: https://coveralls.io/repos/6217/builds

@morozov
Copy link
Member

morozov commented May 23, 2020

Thanks, @greg0ire. From that list, I can navigate to the build for which I'm trying to see the coverage change but having it available as build status would be more convenient.

It looks like I cannot install the Coveralls app from the market because I'm not an organization owner.

@morozov
Copy link
Member

morozov commented May 23, 2020

BTW, not to spoil the fun, the more I look at Codecov, the more I think we should have done a better job comparing the services. E.g. Codecov seems to have nicer code coverage change reporting as a summary as well as provides all the lower-level details as Coveralls does.

At the same time, they charge their customers on the per-user basis which indirectly means they have more manpower to maintain the service. Additionally, they may not have the problem integrating with Appveyor.

@greg0ire
Copy link
Member Author

Well it's still time to change, only the DBAL has Coveralls for now.

@greg0ire greg0ire added this to the 2.10.3 milestone Jun 7, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants