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

Load Kotlin parent first #13445

Merged
merged 2 commits into from
Nov 27, 2020
Merged

Load Kotlin parent first #13445

merged 2 commits into from
Nov 27, 2020

Conversation

stuartwdouglas
Copy link
Member

@stuartwdouglas stuartwdouglas commented Nov 24, 2020

Also change to 1.4, which I am not sure if want just yet or if it should be a different PR (although this is all inter-related, as you can't do 1.4 upgrade without these changes).

Also note that the existing Kotlin tests exhibit the failure from #11549 when the update to 1.4 is performed, so I did not add a specific test for this (as we already have tests).

Fixes #11549

@ghost ghost added area/dependencies Pull requests that update a dependency file area/kotlin labels Nov 24, 2020
@ghost ghost added the area/panache label Nov 24, 2020
Copy link
Member

@evanchooly evanchooly left a comment

Choose a reason for hiding this comment

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

questions aside, this looks great. the load parent first was the missing piece in my own work so this cleans that all up. It does look like the mongodb modules need an update, too, as they are failing with the same "can't find kotlin.Any" error.

@evanchooly
Copy link
Member

I tried pushing changes to the PR but they ended up on my fork instead. https://github.com/evanchooly/quarkus/tree/11549

The diff is pretty light. i'll take another swing at getting them in right place (if I have rights) after lunch.

@stuartwdouglas
Copy link
Member Author

There is some weirdness here, as I can't reproduce these failures locally (even though I could originally).

I then thought my changes had fixed them, but it turns out somehow those tests just started working on their own for me locally, which I still don't understand.

@evanchooly
Copy link
Member

did you see my PR on your PR? just a handful of changes on what's here and all seems to be well.

stuartwdouglas and others added 2 commits November 25, 2020 14:00
Also change to 1.4

Fixes quarkusio#11549
light clean up elsewhere
@stuartwdouglas
Copy link
Member Author

I have updated it

@evanchooly
Copy link
Member

@geoand i'm already on the review. :) I think we're just waiting for CI (which looks like it might be hung with that job already taking 2.5h) and we're good to go. i tested the PR with the reproducer project the OP submitted and it passed.

@geoand
Copy link
Contributor

geoand commented Nov 25, 2020

@geoand i'm already on the review. :) I think we're just waiting for CI (which looks like it might be hung with that job already taking 2.5h) and we're good to go. i tested the PR with the reproducer project the OP submitted and it passed.

👍

I only re-requested the review because you had requested changes and Stuart seems to have addressed them

@evanchooly
Copy link
Member

Got it. He merged a PR i filed against his. Provided the tests pass in all thumbs up.

@geoand geoand merged commit 1a43dbe into quarkusio:master Nov 27, 2020
@ghost ghost added this to the 1.11 - master milestone Nov 27, 2020
@ghost
Copy link

ghost commented Nov 27, 2020

Milestone is already set for some of the items:

We haven't automatically updated the milestones for these items.

This message is automatically generated by a bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/kotlin area/panache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quarkus, Jackson and Kotlin 1.4
3 participants