-
Notifications
You must be signed in to change notification settings - Fork 5
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
Coverage for functions with a macro #67
Comments
Oh, I just saw that the other case in that package https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/src/master/src/ManifoldsBase.jl#487 is even a signature in an if within a macro. But from the first case in the initial post we have quite some cases in the main package (Manifolds.jl), and it would be awesome to have that covered correctly, if possible. |
I think I remember @vtjnash giving advice on this on the past. |
We've fixed some cases of this in Base, though some might still exist. The later example seems to a specific case of that, where the code is only active (due to the if statement) on old versions of Julia, while the fixes for that coverage only exist on newer versions. |
For the later example I am surprised though that the inner lines are covered. But you are right, that seems to be a really specific case. The first example is one, that we have quite often (in Manifolds.jl, luckily only once in ManifoldsBase.jl). |
First of all thanks for this amazing package and all the functionality (and how easy it is to use it), it really helps getting code coverage.
For a while already I was wondering, if some certain cases might be covered as well, so I am not sure whether this is the right repo (I hope so).
We have a few macros for functions, so we have some cases with
See for example
https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/src/master/src/EmbeddedManifold.jl#L190
There, the lines in the body of the function are covered, but the (macro+) signature are not. Can this maybe be fixed/added? That would be great, because for the mentioned package this is the only coverage missing.
The text was updated successfully, but these errors were encountered: