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

Added option to specify diff file location #410

Merged
merged 5 commits into from
Jun 25, 2024

Conversation

Jsostmann
Copy link
Contributor

Added a flag to accept a path which points to a file containing the output of git diff

ex: diff-cover coverage_folder/coverage.xml --html-report report.html --diff-file=diff.txt

@Bachmann1234
Copy link
Owner

Neat, thanks for the PR.

to merge this ill. need tests.

I also think the file tool is strongly dependent on the diff format diff-cover uses. So some documentation on how to generate the diff file in the readme would be nice.

@Jsostmann
Copy link
Contributor Author

No problem. And okay sure thing.
I'll add some tests / documentation and update the PR.

Thanks for the quick reply!

@Jsostmann
Copy link
Contributor Author

@Bachmann1234 Have added documentation and 4 UT for the diff file tool

@Bachmann1234
Copy link
Owner

@Jsostmann looks like you just have to run black over the pr to format things. Then this is looking good!

Thanks!

@Jsostmann
Copy link
Contributor Author

@Jsostmann looks like you just have to run black over the pr to format things. Then this is looking good!

Thanks!

Awesome! and Thanks I'll go ahead and reformat using black.

@Bachmann1234
Copy link
Owner

Ah jeez. My bad. I forgot to mention isort which formats the imports

@Jsostmann
Copy link
Contributor Author

Jsostmann commented Jun 24, 2024

Ah jeez. My bad. I forgot to mention isort which formats the imports

Haha no worries! I should've caught that also. I didn't run verify.sh before pushing

@Bachmann1234
Copy link
Owner

Awesome. Build is passing I should be able to get this merged and released soon. Hoping for tonight

@Jsostmann
Copy link
Contributor Author

Awesome. Build is passing I should be able to get this merged and released soon. Hoping for tonight

Great! No rush, I appreciate the help!

@vser1
Copy link

vser1 commented Jun 25, 2024

Thanks a lot @Jsostmann for the feature and @Bachmann1234 for your great work on this most helpful project!
This PR should close #26 , #150 , and possibly #16 (with some additional client-side work). Looking forward for a 9.1.0! 🎉

@Bachmann1234
Copy link
Owner

Alrighty, lets get this merged. Thanks for the PR @Jsostmann !

@Bachmann1234 Bachmann1234 merged commit 838ffd1 into Bachmann1234:main Jun 25, 2024
7 checks passed
@Bachmann1234
Copy link
Owner

Check out version 9.1.0! https://pypi.org/project/diff-cover/9.1.0/

thanks again

@Jsostmann
Copy link
Contributor Author

Thanks a lot @Jsostmann for the feature and @Bachmann1234 for your great work on this most helpful project!

This PR should close #26 , #150 , and possibly #16 (with some additional client-side work). Looking forward for a 9.1.0! 🎉

Sure thing! Hope it helps!

@Jsostmann
Copy link
Contributor Author

Alrighty, lets get this merged. Thanks for the PR @Jsostmann !

Awesome! Sure thing!

@chkch
Copy link

chkch commented Jul 15, 2024

@Jsostmann May I ask if there is validation for diff file=diff. txt to support Jacoco generated XML full coverage reports? I recently encountered an issue while generating incremental reports based on multiple XML full coverage reports in a Java project.
The execution steps are as follows:
git diff master test > diff.txt
diff-cover report_*.xml --diff-file=diff.txt
Prompt me: No lines with coverage information in this diff

@Jsostmann
Copy link
Contributor Author

Hey @chkch did you check to make sure that the diff file you created had any changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants