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

add display name in invalid and missing to enable better error messages #3567

Closed
wants to merge 0 commits into from

Conversation

motu55
Copy link
Contributor

@motu55 motu55 commented Sep 27, 2023

What's changed?

Invalid Objects have now an recipeDisplayName property to enable better error messages along the way.
Used the new Methods in the basic Recipe.

What's your motivation?

using the maven plugin getting this error message

[ERROR] Recipe validation error in artifactId: is required
[ERROR] Recipe validation error in artifactId: is required
[ERROR] Recipe validation error in artifactId: is required

this is not very helpful since all the errors are actually one error and i had no idea where to look, so had to debug into the maven plugin which is very cumbersome

Anything in particular you'd like reviewers to focus on?

I deprecated the old methods in order to break nothing. Also to not have to move all Existing recipes in the repo to the new methods but can be done if requested.
Also i used the recipe in the constructor and not a string so to get the display name in the constructor.

Have you considered any alternatives or workarounds?

No

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've added the license header to any new files through ./gradlew licenseFormat
  • I've used the IntelliJ IDEA auto-formatter on affected files

@timtebeek
Copy link
Contributor

hi @motu55 ; thanks for looking to help improve the experience when users encounter validation issues. It's indeed not ideal right now, any your proposal is interesting to consider. As also indicated on openrewrite/rewrite-maven-plugin#633 (comment) though there are a lot of unintentional changes in your pull request that make it hard to see what exactly was added, and what was merely moved. Would you be able to reduce the diff to just the intentional changes? That would give us a better understanding of the potential impact.

That said, you are changing the Recipe class which would affect all other (1000+) recipes, and as such we're rather careful with changes there. I can't yet fully see if your changes would be accepted even if cleaned up, so perhaps it's good to have some more context & discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants