native support lcov.info files #297
Conversation
|
|
||
| The path strings may or may not exist. | ||
| """ | ||
| parser = argparse.ArgumentParser(description=DESCRIPTION) | ||
|
|
||
| parser.add_argument("coverage_xml", type=str, help=COVERAGE_XML_HELP, nargs="+") | ||
| parser.add_argument("coverage_file", type=str, help=COVERAGE_FILE_HELP, nargs="+") |
There was a problem hiding this comment.
I'd prefer some attempt to preserve backwards compatibility.
Perhaps including the coverage xml file as an alternative argument but that does the same thing (if both are provided I'm fine with preferring coverage_file. Or maybe throw an error if both are provided?)
Renaming the arg everywhere else makes sense. I just would like the command line to support coverage_xml as an alternate form
There was a problem hiding this comment.
Thanks for the feedback, I'll take care of it.
There was a problem hiding this comment.
btw, can you please enable the workflows for this PR?
There was a problem hiding this comment.
I got a bit confused. This line defines a positional argument (see ArgumentParser.add_argument), therefore nobody will call it with the argument name; it will only define the key in the resulting structure, which is only used internally.
Looking at test_diff_cover_tool.py#L9 shows that nothing changed about how to call the function, therefore there's no backward compatibility issue.
Can you please double-check if my understanding is correct?
There was a problem hiding this comment.
Yeah, I think you are right. Sorry. Misread the argparse code :-)
Support lcov.info files, as described in https://ltp.sourceforge.net/coverage/lcov/geninfo.1.php
Potential future improvements: let users mix lcov.info and xml reports.