Skip to content

Introduce relative coverage change info #40

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.

Sign up for GitHub

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

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

Introduce relative coverage change info #40

paescuj wants to merge 2 commits into from

Conversation

@paescuj
Copy link
Contributor

@paescuj paescuj commented Oct 28, 2022 •
edited
Loading

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

MishaKav commented Oct 30, 2022

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

MishaKav commented Oct 30, 2022

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)

MishaKav
MishaKav requested changes Nov 1, 2022
View reviewed changes
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?

@kingsldn
Copy link

kingsldn 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 •
edited
Loading

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

@kingsldn
Copy link

kingsldn commented Feb 25, 2023

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

@kingsldn
Copy link

kingsldn 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

mindrunner commented May 27, 2023

This would be very nice to have :)

@aviralpostman
Copy link

aviralpostman commented May 23, 2024

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

@aviralpostman aviralpostman mentioned this pull request Jun 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MishaKav MishaKav MishaKav requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@paescuj @MishaKav @kingsldn @mindrunner @aviralpostman