search

JuliaCI / Coverage.jl

Take Julia code coverage and memory allocation results, do useful things with them
MIT License
174 stars 68 forks source link

RFC: Factor out the core functionality to CoverageTools.jl #249

Closed DilumAluthge closed 4 years ago

DilumAluthge commented 4 years ago

This PR factors out the core functionality of Coverage.jl to the CoverageTools.jl package.

Since this is a major change to a package that is used by many people, it would be good to get as many people as possible to review this PR.

Please note: since this is such a big change, I bumped the version number from 0.9.3 to 1.0.0-DEV, because I am not sure that nothing in this PR is breaking. However, the reviewers feel confident that this PR is not breaking, we can instead bump the version number from 0.9.3 to 0.9.4-DEV.

codecov-io commented 4 years ago

Codecov Report

Merging #249 into master will decrease coverage by 6.74%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #249      +/-   ##
==========================================
- Coverage   90.31%   83.57%   -6.75%     
==========================================
  Files           6        3       -3     
  Lines         351      140     -211     
==========================================
- Hits          317      117     -200     
+ Misses         34       23      -11
Impacted Files Coverage Δ
src/coveralls.jl 81.08% <ø> (ø) :arrow_up:
src/Coverage.jl 100% <ø> (+1.86%) :arrow_up:
src/codecovio.jl 86.15% <ø> (ø) :arrow_up:

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 5f811e7...40597f5. Read the comment docs.

davidanthoff commented 4 years ago

If the final verdict is that this PR is breaking, could the version be increased to 1.0.0?

fingolfin commented 4 years ago

This PR says nothing about the motivation for doing it...

DilumAluthge commented 4 years ago

We've registered the package CoverageCore.jl (https://github.com/JuliaCI/CoverageCore.jl).

CoverageCore has the functionality for processing the code coverage and memory allocation results. It only has one dependency (the LibGit2 stdlib); it has no non-stdlib dependencies. Thus, people that want to use the functionality of CoverageCore in their own packages can do so without needing to take on any additional dependencies.

Coverage.jl will continue to have the functionality for uploading coverage reports to Codecov.io and Coveralls.io.

However, now that we have CoverageCore, we have the same code duplicated in two places, which will be harder to maintain in the long run. So this PR simply removes the duplicated code that has already been moved to CoverageCore.

davidanthoff commented 4 years ago

One place where this will be super helpful is the Julia VS Code extension: we could ship CoverageCore.jl and thus provide out of the box support for generating lcov files. We can’t do that with the current Coverage.jl because we can’t ship anything that has binary dependencies.

DilumAluthge commented 4 years ago

Bump.

It would be great to get as many eyes on this as possible.

I will leave this PR open for 2 more weeks, and then (assuming that nobody spots any bugs) will merge it.

vtjnash commented 4 years ago

My thought would be just giving it a more descriptive name perhaps. Perhaps CoverageIO or CoverageTools?

DilumAluthge commented 4 years ago

My thought would be just giving it a more descriptive name perhaps. Perhaps CoverageIO or CoverageTools?

Sure, we can rename CoverageCore. I think CoverageTools would be a good name.

Since CoverageCore is already registered in the General registry, I just need to figure out the process for renaming it.

DilumAluthge commented 4 years ago

CoverageCore has been renamed to CoverageTools.