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

Upgrade Maven resolver related dependencies used by bootstrap #7459

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

aloubyansky
Copy link
Member

to those used by/compatible with Maven 3.6.3 release

Fixes #3539
Fixes #7458

…e used by/compatible with Maven 3.6.3 release
@aloubyansky aloubyansky requested a review from gsmet February 27, 2020 15:05
@aloubyansky aloubyansky added this to the 1.3.0 milestone Feb 27, 2020
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Looks good to me. Asked @geoand to check the Spring change.

final ClassInfo classInfo = index.getClassByName(clazz);
if (classInfo != null) {
ret.add(classInfo);
}
Copy link
Member

Choose a reason for hiding this comment

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

@geoand can you check that change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I saw an NPE there.

Copy link
Member Author

Choose a reason for hiding this comment

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

I saw an NPE there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, not at this line exactly.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Weird NPE, have never seen it to there. Check can't hurt of course. I'll keep an eye out for any issues that might be related to Spring DI.

@aloubyansky
Copy link
Member Author

If you want to reproduce it @geoand, take this commit an undo that particular change.

@geoand
Copy link
Contributor

geoand commented Feb 27, 2020

I'll do that now

@geoand
Copy link
Contributor

geoand commented Feb 27, 2020

@aloubyansky which test failed? I can't seem to reproduce

@aloubyansky
Copy link
Member Author

I think it was in spring-di deployment.

@aloubyansky
Copy link
Member Author

or in the IT. Sorry, don't remember for sure. Do you want me to reproduce it for you? I even remember logging the annotation. I think it was javax.annotation.Nonnull.

@geoand
Copy link
Contributor

geoand commented Feb 27, 2020

I tried the deployment module and all Spring ITs and nothing tripped...

@aloubyansky
Copy link
Member Author

Ok, I'll try again.

@geoand
Copy link
Contributor

geoand commented Feb 27, 2020

Oh, nevermind, I was able to reproduce it now

@geoand
Copy link
Contributor

geoand commented Feb 27, 2020

OK, fix is good and should have been there in the first place :).

Thanks @aloubyansky

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants