Skip to content

Conversation

@cmelchior
Copy link
Contributor

Closes #7615
Replaces #7616

Copy link
Contributor

@rorbech rorbech left a comment

Choose a reason for hiding this comment

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

LGTM. Only minor comment about the sort order. But maybe worth to check if the same consideration apply when iterating other containers, i.e. properties?

for (metadata in classesToValidate) {
// Sort classes to ensure deterministic output. This is relevant when e.g. using Gradle
// Remote Cache since the order is not guaranteed between OS and Java versions.
for (metadata in classesToValidate.toSortedSet(compareByDescending { it.qualifiedClassName.name })) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason for sorting this with compareByDescending instead of just compareBy? Just makes it a bit annoying to read the expected output, but I guess we can then see it is actually affecting the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I assume that the reason was that we could see it affect the output. Which it did, since it broke our unit test. But yes, sorting is otherwise arbitrary.

Properties seem to be processed in order from their class, which looks fine.

@cmelchior cmelchior merged commit a9277eb into master Dec 17, 2021
@cmelchior cmelchior deleted the cm/bug/deterministic-annotation-processor-output branch December 17, 2021 09:24
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent order in annotation processing

4 participants