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

Don't enforce sorting of Spring resource locations #204

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

Ptijohn
Copy link
Contributor

@Ptijohn Ptijohn commented Aug 19, 2022

@Ptijohn Ptijohn requested review from Stephan202, werli and rickie August 19, 2022 10:35
"javax.xml.bind.annotation.XmlType#propOrder");
"javax.xml.bind.annotation.XmlType#propOrder",
"org.springframework.test.context.TestPropertySource#locations",
"org.springframework.test.context.TestPropertySource#value");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

value and locations are aliases for each other:
image

Copy link
Member

Choose a reason for hiding this comment

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

There's also @PropertySource; will add it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @Stephan202 !

@Stephan202 Stephan202 force-pushed the bdiederichs/ANS-1117 branch from cff1ed2 to cd337d8 Compare August 19, 2022 12:39
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Rebased and added a commit :)

Congrats on your first Error Prone Support contribution! 🎉

"javax.xml.bind.annotation.XmlType#propOrder");
"javax.xml.bind.annotation.XmlType#propOrder",
"org.springframework.test.context.TestPropertySource#locations",
"org.springframework.test.context.TestPropertySource#value");
Copy link
Member

Choose a reason for hiding this comment

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

There's also @PropertySource; will add it :)

@@ -144,7 +145,13 @@ void identification() {
" A secondEndpoint();",
"",
" @XmlType(propOrder = {\"field2\", \"field1\"})",
" class Dummy {}",
" class FirstDummy {}",
Copy link
Member

Choose a reason for hiding this comment

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

Let's use names that make future reordering less likely 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :)

@Stephan202 Stephan202 added this to the 0.1.1 milestone Aug 19, 2022
@Stephan202
Copy link
Member

Stephan202 commented Aug 19, 2022

Suggested commit message:

Don't enforce sorting of Spring resource locations (#204)

Because their order matters.

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Nice @Ptijohn , thanks for opening the PR 🚀 !

Made one tiny tweak in the suggested commit message. Otherwise it sounds more like a general statement to me.

@Stephan202
Copy link
Member

@rickie not clear what "there" refers to. Proposed something else.

@Ptijohn
Copy link
Contributor Author

Ptijohn commented Aug 19, 2022

Thanks @Stephan202 and @rickie for the fast review + input!
Not sure what's the merging process, so I'll let you do what you do ;)

@rickie rickie changed the title Exempt @TestPropertySource#locations/value from being sorted. Don't enforce sorting of Spring resource locations Aug 19, 2022
@rickie rickie merged commit e64af1d into master Aug 19, 2022
@rickie rickie deleted the bdiederichs/ANS-1117 branch August 19, 2022 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants