siom79 / japicmp

Comparison of two versions of a jar archive
https://siom79.github.io/japicmp
Apache License 2.0
694 stars 107 forks source link

New option to make reports less verbose `--report-only-summary` #398

Closed guillermocalvo closed 1 week ago

guillermocalvo commented 3 months ago

Sometimes compatibility reports may be a bit too verbose, especially if all you need is the list of incompatible classes.

I would like to introduce a new option --report-only-summary that will omit further details when generating plain text or html reports. I think it shouldn't affect xml reports, because they're not meant for direct human consumption.

It should be disabled by default, so reports continue to show all the available information unless the option is explicitly enabled.

siom79 commented 2 months ago

@guillermocalvo Thank you for work and for thinking about how to improve the tool.

The basic idea of showing less information is not bad. What I have had in mind for longer and what also has been requested some time ago would be a more advanced filtering solutions for the reports. Only skipping methods and fields by one flag might be a good idea. But what about a more fine-grained filtering mechanism, allowing users to choose what to see in the report?

guillermocalvo commented 2 months ago

@siom79 Sure, I can take a stab at implementing a more fine-grained mechanism.

Just so we are on the same page; when you talk about "filtering solutions", you mean being able to omit just fields, or just constructors and annotations, etc.?

If that's the use case, we could define new options:

So users would be able to select any combination thereof to exclude those specific details from the report.

Is something like that what you had in mind?

siom79 commented 2 months ago

The point here is that we currently propagate the incompatibility from the lower elements up to the class. Meaning that if you modify a method incompatible the class is marked as being changed incompatible. If we now hide the method, the class is still shown as incompatible, but you do not know why.

We already have flags for only binary incompatible changes and any changes.

So if we filter out methods and fields we should indicate somehow that incompatible changes come from the not shown methods and or fields.

I would also prefer only one flag with a list like --hide-in-report=methods,fields,constructors.

guillermocalvo commented 2 months ago

The point here is that we currently propagate the incompatibility from the lower elements up to the class. Meaning that if you modify a method incompatible the class is marked as being changed incompatible. If we now hide the method, the class is still shown as incompatible, but you do not know why.

Yes, that's precisely my use case: sometimes I want to know which classes are incompatible but I don't care why they are incompatible. Let me illustrate it with an example:

  1. On the one hand, I may want to automatically generate "summary only" reports for each pull request, so that I can know which ones contain breaking changes and which classes are going to be affected (should the PR be merged).
  2. On the other hand, I may want to generate a "detailed" report when I release a new version, to fully document what classes are incompatible and how exactly they changed.

We already have flags for only binary incompatible changes and any changes.

My intent was that --report-only-summary wouldn't affect what's considered compatible and what's considered incompatible. The new option was meant to simply hide the details from the report. Everything else (including --error-on-xxx options) should keep working as usual.

So if we filter out methods and fields we should indicate somehow that incompatible changes come from the not shown methods and or fields.

I just wanted to offer a mechanism to keep reports short and concise for those situations when there's no need for full details. I didn't think a warning was really necessary, because users must enable the option explicitly.

🤔 I think maybe my specific use case doesn't have much in common with your advanced filtering feature, after all. What do you think @siom79 ?

guillermocalvo commented 2 weeks ago

Hi @siom79 👋

Have you had a chance to think about what we should do with this new feature?

siom79 commented 1 week ago

Hi @guillermocalvo ,

ok, after thinking some time about it :wink: there is a chance that this PR gets merged. But why is it only implemented for the CLI and ant task and not for the maven plugin?

guillermocalvo commented 1 week ago

@siom79 You're right; I missed that part 😅

I have just pushed some changes to add a new parameter reportOnlySummary to the Maven plugin. Please let me know if there's anything else I may have overlooked. Thank you!

siom79 commented 1 week ago

Released with 0.22.0.