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

New option to generate reports in Markdown format --markdown #405

Merged
merged 15 commits into from
Aug 20, 2024

Conversation

guillermocalvo
Copy link
Collaborator

Solves #404

Note

An example Markdown report generated for demonstration purposes can be found here.

@siom79 Sorry for the big PR 😅 I tried to make this new functionality as customizable as possible. Please feel free to edit it or suggest any changes you think should be done before it can be merged.

@scordio scordio mentioned this pull request Aug 2, 2024
@scordio
Copy link
Contributor

scordio commented Aug 2, 2024

Very cool work @guillermocalvo, thanks for taking this up!

Copy link
Owner

@siom79 siom79 left a comment

Choose a reason for hiding this comment

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

Excellent work! Thanks very much.

src/site/markdown/Examples.md Outdated Show resolved Hide resolved
japicmp/src/main/java/japicmp/cli/JApiCli.java Outdated Show resolved Hide resolved
Full explanation:
JDK 8 doesn't realize we want to handle a stream of `JApiCompatibility`
elements. Instead, the compiler thinks we want to create a stream of
`JApiHasChangeStatus` elements. So I just extracted the complex
expression to a constant with the appropriate type and it works fine.

Other JDK versions get it right without the type hint. ¯\_(ツ)_/¯

Root cause of the problem:

```
Caused by: java.lang.invoke.LambdaConversionException: Invalid receiver type interface japicmp.model.JApiHasChangeStatus; not a subtype of implementation type interface japicmp.model.JApiCompatibility
	at java.lang.invoke.AbstractValidatingLambdaMetafactory.validateMetafactoryArgs(AbstractValidatingLambdaMetafactory.java:233)
	at java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:303)
	at java.lang.invoke.CallSite.makeSite(CallSite.java:302)
	... 54 more
```
@guillermocalvo
Copy link
Collaborator Author

Excellent work! Thanks very much.

Thanks for your review @siom79! I have implemented your suggestions.

Incidentally, the JDK 8 build failed because the JVM was confused about some Java types 🤔 It's weird, because it was working perfectly fine two weeks ago, so I think the CI worker must be running a different version now. In any case, I extracted an expression to a constant to make sure the compiler understands what the types in play are.

@siom79 siom79 merged commit 554a6c6 into siom79:master Aug 20, 2024
7 checks passed
@siom79
Copy link
Owner

siom79 commented Aug 20, 2024

Thanks @guillermocalvo for implementing the changes. I have resolved the two last remarks myself and merged the result to main.

Btw: Are you interested to become a collaborator on this repository?

@guillermocalvo guillermocalvo deleted the markdown branch August 20, 2024 21:26
@guillermocalvo
Copy link
Collaborator Author

Btw: Are you interested to become a collaborator on this repository?

@siom79 Sounds great! Is there anything specific I can help with?

@siom79
Copy link
Owner

siom79 commented Aug 22, 2024

@guillermocalvo I have sent you an invitation. Currently there is nothing specific, but as contributor you can merge PRs and do other helpful things. But please be cautious with the new power. 😏

@guillermocalvo
Copy link
Collaborator Author

@siom79 Thanks! And don't worry, I know that "with great power comes great responsibility". 😉

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.

3 participants