Skip to content

Add markdown report & badge #90

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

easazade
Copy link

@easazade easazade commented Jan 17, 2024

This PR updates the report command to also generate a markdown report and a github badge based on the given thresholds.

markdown report is a one page compact report with a short summary that can be used in pull requests to show a short report of the code coverage

example of the generated markdown reprot
Screenshot 2024-01-17 at 14 22 19

@easazade easazade requested a review from mrverdant13 as a code owner January 17, 2024 13:14
@easazade
Copy link
Author

@mrverdant13 can you review please when you have the time?

@mrverdant13
Copy link
Owner

Hi, @easazade 👋🏻
Thanks for contributing!
I'll take a look asap (I'm currently in a rural area without reliable connection)

@mrverdant13
Copy link
Owner

Hi again, @easazade

I've been thinking of this feature.
I really believe it can be quite useful and, first, I'd like to talk about some points to take in mind.

Concern

My main concern about the current implementation is that this feature is not optional when generating reports.
Consider that not all devs may be interested in this particular functionality.
This also applies for the HTML reports, but they are already a step that can be ignored just by not using the report command.

Another potential usability hustle that may arise is the increased complexity that would be introduced in a single command that was initially created for a single purpose. This would result in a set of optional command options and flags with cumbersome relationships, constraints and conditions, that would definitely hinder testability.

So, I am open to discuss a new approach that may handle this with a better approach.


Proposal

Using report sub-commands

One alternative we may consider is the use of sub-commands, one per each type of report (HTML and markdown, at the moment).

The expected API could be used as follows:

  • coverde report html [OPTIONS]
  • coverde report markdown/md [OPTIONS]
  • coverde report custom [OPTIONS] (potential new feature to support more complex use cases that can be tailored as desired).

PROS:

  • Each sub-command can have discriminated options that may apply to the proper report format. Some examples: a) For markdown reports, users may pick custom emojis. b) For HTML reports, custom colours may be selected.
  • The fact that each format is isolated in its own sub-comand, allow users to pick them conditionally depending on their needs.
  • If the custom variant is implemented, we can delve in a flexible method to create custom reports without requiring CLI updates or explicit support for some particular use cases.
  • If the custom variant is implemented, we could even improve the dev experience for the CLI itself when including new formats.
  • For projects with standardized workflows, we could even support a coverde.yaml config file that defines the options for each format type, and a variant coverde_overrides.yaml config file (that shouldn't be included in source control) would be handy for local development, as well. These files would match the discriminated options mentioned in the first point.
  • Reliable maintainability and testability with predictable test cases.

CONS:

  • Multiple report formats wouldn't be generated with a single command call. Each of them would require an explicit command invocation, which may be quite slow for large projects (specially those with mono-repo setups).
  • This would be a breaking change, otherwise, we could also support the current HTML report generation behaviour for the umbrella coverde report command, just for backwards compatibility.
  • I would strongly argue that the custom variant should be the priority, as it would define the foundational behaviour for this feature.

As a side note, I understand the functionality that this PR holds may be a priority for you.
I hope you understand the points I listed above as the reasons behind why I wouldn't approve this PR just yet.

@easazade
Copy link
Author

easazade commented Jan 23, 2024

@mrverdant13 Thanks for the review. I can add sub commands html & markdown but what do you have in mind for the custom sub command ? I think for custom reports we can require an option for path of a dart script with a certain function signature to be provided. Then the implementation of the function that generates the report base on the metadata that coverede provides will be on developer's side.

In general I would suggest not to add a lot of stuff to the scope of this PR. but it's your repo you make the decision.

@mrverdant13
Copy link
Owner

@easazade
Totally agree on that
Implementing the html and markdown sub-commands would be enough, for now.
If you can take that, it would be awesome.
Again, thanks for the contribution!

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.

2 participants