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

Make @Issue repeatable #4049

Merged
merged 1 commit into from
Feb 25, 2024
Merged

Conversation

Bananeweizen
Copy link
Contributor

What's changed?

Allow multiple issue annotations for a test.

What's your motivation?

I once used that accidentally because I have it in other projects, and had to fix the PR after I noticed the compile error.

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

It uses a nested interface for the container class to avoid it being shown in typical code completion of the IDE.

Use a nested interface for the container class to avoid it being shown
in typical code completion of the IDE.
@timtebeek
Copy link
Contributor

I'd considered this the other day as well; thanks opening a PR! An alternative I'd considered was using String[] value to keep just the same single annotation for most cases. Any thoughts on that?

@timtebeek timtebeek added documentation Improvements or additions to documentation enhancement New feature or request labels Feb 25, 2024
@Bananeweizen
Copy link
Contributor Author

I find copying the entire line and replacing the url more simple than adding a second URL to an existing tag (and quite honestly, I also like reading 2 similarly formatted lines more than the one tag spread over 2 lines or both URLs in the same line). But that surely is just personal preference, I had similar discussions also with colleagues and we typically see at least as many opinions as people involved, if not more. :)

I'm not sure if there are any good arguments for/against one or the other from the view point of binary compatibility, usability with reflection or other more technical aspects. I've also not checked if there are other custom tags in your code base, where we might want to align. So if you have some strong opinion for either one, we can surely change this.

@timtebeek
Copy link
Contributor

Thanks for laying out your thoughts; no strong opinions here, so then let's indeed got for repeated annotations.

@timtebeek timtebeek merged commit 52454c4 into openrewrite:main Feb 25, 2024
1 check passed
@Bananeweizen Bananeweizen deleted the issue_repeatable branch February 25, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants