Skip to content

Conversation

@Pankraz76
Copy link
Contributor


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@Pankraz76 Pankraz76 force-pushed the LexicographicalAnnotationListing branch from d833672 to 509921a Compare October 13, 2025 09:51
@sormuras
Copy link
Member

In general, I am not opposed to sort elements by name. But seeing @Retention and @Target split apart, with for example @SuppressWarning in between, is ... odd. In book, they belong together. Logically.

I also like @API being the closest to the actual type declaration.

@Pankraz76
Copy link
Contributor Author

Pankraz76 commented Oct 13, 2025

Yes, it's subjective. At least there would be a clear order, some starting with JSON would then be grouped, but it's up to the community to decide each rule.

Checkstyle has both in place, so it's only somebody's darling.

Also, this seems to be the last of the fixable rules somehow all others not apply here.

Will update then as rejected.

Thanks.

@sbrannen
Copy link
Member

I will not accept any automated sorting of annotations.

We intentionally group them logically.

Furthermore, annotation declaration matters and is retained in the byte code, and that is guaranteed in the JLS.

Granted, the order of the annotations modified in this PR may not matter in terms of semantics, but annotation order in general can matter for certain use cases.

@Pankraz76, it would be beneficial if you would first ask if the team desires such changes before spending time implementing something that may get rejected. If I recall correctly, we have told you this before, but you seem to ignore the advice for some reason.

@Pankraz76
Copy link
Contributor Author

@Pankraz76, it would be beneficial if you would first ask if the team desires such changes before spending time implementing something that may get rejected. If I recall correctly, we have told you this before, but you seem to ignore the advice for some reason.

Yes im sorry, please excuse the amount of notifications.

We have considered to evaluate the remaining checks.

I will slow down, as the interesting ones like ConstantNaming does not work apply the patch here currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants