WIP: Initial cargo-grcov command #533
Conversation
|
Sorry for the delay, I hope I'll be able to get to this by the end of this week. |
(until coverage is available on stable)
|
Is there any progress on this? I'm recently using grcov a lot and it would be great to have a dedicated |
|
Unfortunately I didn't have time to review this :( If somebody can help me with reviewing the code in detail, I can review the overall architecture. (Assuming @gilescope is still interested in finishing this). |
|
I looked through the code and it is pretty much a wrapper around the I personally prefer to have Pretty much what was done for the flamegraph tool is what I think is more effective and safer towards errors. As we have a good abstraction already, it's easy to implement as well. If you, @marco-c, don't mind, I would start a new PR that uses the same technique as in the flamegraph crate and combine it with the effort already done here, then adding in more features. For that PR I would probably strip many options from the CLI and only expose what make sense in the context of cargo. What do you think? |
|
@dnaka91 sounds good to me! |
|
Sounds good to me too - I just want to use it :-) so happy to test out,
feedback etc.
…On Tue, 29 Jun 2021 at 10:06, Marco Castelluccio ***@***.***> wrote:
@dnaka91 <https://github.com/dnaka91> sounds good to me!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#533 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGEJCEGEHLHHZEYKCMVXOTTVGEIRANCNFSM4UKIMFHA>
.
|
|
@dnaka91 still interested? |
|
Definitely! Sorry, I didn't post any update on this for a while. Had a few other things going on and simply didn't get to it yet. Already checked the current implementation of this PR and I think I'll go with a very similar approach and then add a few cli args from the original grcov binary. Also, I was hoping that #648 would maybe be merged before I start on this (or decided not to, just so I don't have to implement the CLI logic twice). Is there any chance you could give it a look soonish? 🙇♂️ |
|
@dnaka91 structopt is merged in now if that helps... |
|
I've updated this PR to structopt. @dnaka91 as I said if you want to go on from here that's cool with me. |
|
@gilescope thanks for letting me know. I didn't work on this for quite a long time but will likely be able to pick up on this again soon-ish. |
|
Great news. Happy to test out and give feedback.
…On Sun, 17 Oct 2021 at 16:10, Dominik Nakamura ***@***.***> wrote:
@gilescope <https://github.com/gilescope> thanks for letting me know. I
didn't work on this for quite a long time but will likely be able to pick
up on this again soon-ish.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#533 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGEJCASSVKTDIPKCHJJVD3UHLRPJANCNFSM4UKIMFHA>
.
|
|
@dnaka91 @gilescope does any of you want to pick this up again? |
|
Sorry, I didn't do any further work on this. Reason is, that I moved to |
PR for cargo-grcov. Will fix #326
I'm a big fan of early review to make sure I'm headed in the right direction. Wasn't qute sure how grcov created errors - they seem to be logged with log.error!(), but didn't spot any custom errors - should I just create a specific error type where needed - or is there one that converts from a string?