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

Introduce relative coverage change info #40

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

paescuj
Copy link
Contributor

@paescuj paescuj commented Oct 28, 2022

The report will now include information about the relative change in coverage when the previous coverage value is passed along with the file input.
For example, if the coverage info is stored in a gist (for the badge), this value can be read out and passed to this action.

coverage-summary-path: ./coverage/coverage-summary.json, 60
multiple-files: |
  title1, ./data/coverage_1/coverage-summary.json, 60
  title2, ./data/coverage_1/coverage-summary_2.json, 90

ToDo:

  • Update documentation

Example with single file

Lines Statements Branches Functions
Coverage: 78%
■ Unchanged
76.74% (33/43) 100% (0/0) 33.33% (2/6)

Example with multiple files

Title Lines Statements Branches Functions
title1 undefined: 78%
▲ Increased (+18%)
76.74% (33/43) 100% (0/0) 33.33% (2/6)
title2 undefined: 79%
▼ Decreased (-11%)
77.27% (34/44) 100% (0/0) 33.33% (2/6)

@paescuj paescuj changed the title Introduce ability to pass previous coverage value to get relative info Introduce ability to pass previous coverage value and get relative info Oct 28, 2022
@paescuj paescuj changed the title Introduce ability to pass previous coverage value and get relative info Introduce relative coverage change info Oct 28, 2022
@MishaKav
Copy link
Owner

Thanks for contributing, it's a great feature 💪
I will wait for you to merge from main and resolve the conflicts, so I can see the actual changes.

@MishaKav
Copy link
Owner

Maybe this change may be represented in one line?

Example with multiple files

Title Lines Statements Branches Functions
title1 undefined: 78%(+18%) 76.74% (33/43) 100% (0/0) 33.33% (2/6)
title2 undefined: 79%(-11%) 77.27% (34/44) 100% (0/0) 33.33% (2/6)

@paescuj
Copy link
Contributor Author

paescuj commented Oct 31, 2022

Thanks for your positive feedback!

Maybe this change may be represented in one line?

I like this idea! 👍 I'm going to adjust it accordingly.

@paescuj
Copy link
Contributor Author

paescuj commented Nov 1, 2022

@MishaKav Which of these two variants do you like more?

Title Lines Statements Branches Functions
title1 undefined: 79% ■ (±0%) 77.27% (34/44) 100% (0/0) 33.33% (2/6)
title2 undefined: 79% ▲ (+18%) 77.27% (34/44) 100% (0/0) 33.33% (2/6)
title3 undefined: 79% ▼ (-11%) 77.27% (34/44) 100% (0/0) 33.33% (2/6)
Title Lines Statements Branches Functions
title1 undefined: 79%■ (±0%) 77.27% (34/44) 100% (0/0) 33.33% (2/6)
title2 undefined: 79%▲ (+18%) 77.27% (34/44) 100% (0/0) 33.33% (2/6)
title3 undefined: 79% ▼ (-11%) 77.27% (34/44) 100% (0/0) 33.33% (2/6)

I don't quite like the first variant because the text is not centered vertically along with the badge.
I was able to fix that in the second variant. But as you can see, it now has a space in between...

@MishaKav
Copy link
Owner

MishaKav commented Nov 1, 2022

Which of these two variants do you like more?

I like more the second way, even this with space.

I think another improvement may be to represent the Increased with some green icon (example 🟢) and Decreased with red icon (example 🔴) because they usually represent these metrics (but no sure, if it's better)

Copy link
Owner

@MishaKav MishaKav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also please update a README and action.yaml with example and a screesnhot?

__tests__/summary.test.ts Show resolved Hide resolved
__tests__/summary.test.ts Show resolved Hide resolved
@khause
Copy link

khause commented Feb 7, 2023

This is some great work!
Can we get this in and a new version released please?

@paescuj
Copy link
Contributor Author

paescuj commented Feb 8, 2023

Sorry, haven't been able to finish this over the last weeks! I'll try to do so this weekend 👍

@khause
Copy link

khause commented Feb 25, 2023

Could I help in some way to get this over the line?

@khause
Copy link

khause commented Apr 20, 2023

@paescuj @MishaKav is there anything I can do to help this PR get completed?

@paescuj
Copy link
Contributor Author

paescuj commented Apr 20, 2023

Hmm thanks I had some different ideas for the final implementation, but I have to take a look at it again. I really intend to complete this pull request, hopefully I'll have time to do so this weekend 👍

@mindrunner
Copy link

This would be very nice to have :)

@aviralpostman
Copy link

this is a cool feature. Any updates on when this will be completed/merged?

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.

5 participants