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

Nitrite native support #1298 #2254

Merged
merged 1 commit into from
Feb 25, 2021
Merged

Conversation

JiriOndrusek
Copy link
Contributor

fixes #1298

requires quarkus 1.13 (requires change quarkusio/quarkus#15043)

[ ] An issue should be filed for the change unless this is a trivial change (fixing a typo or similar). One issue should ideally be fixed by not more than one commit and the other way round, each commit should fix just one issue, without pulling in other changes.
[ ] Each commit in the pull request should have a meaningful and properly spelled subject line and body. Copying the title of the associated issue is typically enough. Please include the issue number in the commit message prefixed by #.
[ ] The pull request description should explain what the pull request does, how, and why. If the info is available in the associated issue or some other external document, a link is enough.
[ ] Phrases like Fix # or Fixes # will auto-close the named issue upon merging the pull request. Using them is typically a good idea.
[ ] Please run mvn process-resources -Pformat (and amend the changes if necessary) before sending the pull request.
[ ] Contributor guide is your good friend: https://camel.apache.org/camel-quarkus/latest/contributor-guide.html

Copy link
Contributor

@ppalaga ppalaga left a comment

Choose a reason for hiding this comment

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

Some suggestions inline.

@JiriOndrusek JiriOndrusek force-pushed the 1298-nitrite-native branch 2 times, most recently from 42c8d44 to 1e75bdd Compare February 16, 2021 12:26
@ppalaga
Copy link
Contributor

ppalaga commented Feb 19, 2021

@JiriOndrusek could you please check the test failures?

@JiriOndrusek
Copy link
Contributor Author

@ppalaga I'll look into it

@JiriOndrusek
Copy link
Contributor Author

@ppalaga Failures will go away, once we switch quarkus to 1.13 (requires change quarkusio/quarkus#15043)

@JiriOndrusek
Copy link
Contributor Author

Last comment is wrong, test should succeed also in 1.12.final. I've fixed it.

Copy link
Contributor

@aldettinger aldettinger left a comment

Choose a reason for hiding this comment

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

Nice work @JiriOndrusek Finger crossed that the build issue is easy to get fixed :)

@ppalaga
Copy link
Contributor

ppalaga commented Feb 23, 2021

@JiriOndrusek could you please rebase and re-target at quarkus-master branch so that we do not merge to master by any chance before we have Quarkus 1.13?

@ppalaga ppalaga changed the base branch from master to quarkus-master February 23, 2021 09:59
@ppalaga
Copy link
Contributor

ppalaga commented Feb 23, 2021

OK, I was able to change the base branch from master to quarkus-master myself. Please rebase and change to native-since: 1.8.0

@JiriOndrusek
Copy link
Contributor Author

@ppalaga I'll look into it

@JiriOndrusek
Copy link
Contributor Author

@ppalaga I've probably caused misunderstanding with my last comment (so it probably wasn't seen)
This extension works also with quarkus 1.12.0.Final. I'm rebasing it back to master (or is the reason of your rebase other then my wrong comment?)

@JiriOndrusek JiriOndrusek changed the base branch from quarkus-master to master February 24, 2021 15:01
@JiriOndrusek
Copy link
Contributor Author

I'll fix current failures. I wrongly solved conflicts.

@ppalaga
Copy link
Contributor

ppalaga commented Feb 24, 2021

This extension works also with quarkus 1.12.0.Final. I'm rebasing it back to master

Yes, that's right. I was not aware that your fix is available in Quarkus 1.12 too. Sorry for the confusion from my side.

@JiriOndrusek
Copy link
Contributor Author

God to merge.

@ppalaga ppalaga merged commit d11a894 into apache:master Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nitrite native support
4 participants