-
Notifications
You must be signed in to change notification settings - Fork 5k
[CAMEL-18156] Add support for hyperledger-aries #7973
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
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good, only minor findings related to ASF headers and version
"title": "Aries", | ||
"description": "Access market data and trade on Bitcoin and Altcoin exchanges.", | ||
"deprecated": false, | ||
"firstVersion": "3.17.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First version should be 3.19.0 I suppose, because 3.18.0 is on vote already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -0,0 +1,44 @@ | |||
/*- | |||
* #%L |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section should be removed from the license header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These headers are generated with the maven plugin. I suppose you don't manually maintain license headers across all files. I can remove all headers with plugin and perhaps then regenerate in the way you want it, but how would I do that?
@@ -0,0 +1,152 @@ | |||
/*- | |||
* #%L |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
@@ -0,0 +1,101 @@ | |||
/*- | |||
* #%L |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
@@ -0,0 +1,101 @@ | |||
/*- | |||
* #%L |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
@@ -0,0 +1,112 @@ | |||
/*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
❌ Finished component verification: 1 component(s) test failed out of 2 component(s) tested |
These headers are generated with the maven plugin. I suppose you don't manually maintain license headers across all files. I can remove all headers with plugin and perhaps then regenerate in the way you want it, but how would I do that? Anything I can do to unlock this? |
@tdiesler maybe you can run |
370cf0f
to
1d43092
Compare
@zhfeng Merci, that seems to have worked |
We need to check how the plugin manages the license. But not your fault, so you could ignore my comment. |
checkstyle passes for me locally
|
The CI failure looks like not related to code styles. I think it should be good.
@tdiesler can you check the build.log on https://github.com/apache/camel/actions/runs/2655649560? The compilation error is
|
@zhfeng It's here https://github.com/tdiesler/camel/blob/CAMEL-18156/core/camel-api/src/generated/java/org/apache/camel/Category.java#L55 Perhaps that file is indeed generated and I should define that new category elsewhere? |
@tdiesler I think we need to add |
❌ Finished component verification: 3 component(s) test failed out of 5 component(s) tested |
1 similar comment
❌ Finished component verification: 3 component(s) test failed out of 5 component(s) tested |
❌ Finished component verification: 1 component(s) test failed out of 2 component(s) tested |
This is now failing for some unknown reason
|
I just noticed that the library is not on Maven central. Is that for a particular reason? We already had a lot of troubles with 3rd party Maven repositories like Atlassian.. So I'd really prefer to have the lib on Maven central. |
Do you mean the |
Yes, I mean 3rd party repositories in general. All of them are less reliable than central. It's not really a problem, but at some point I'd move the libraries there |
❌ Finished component verification: 1 component(s) test failed out of 2 component(s) tested |
sure, we can do that when needed |
✔️ Finished component verification: 0 component(s) test failed out of 2 component(s) tested |
✔️ Finished component verification: 0 component(s) test failed out of 3 component(s) tested |
I'll try to merge this week. |
This is now failing for some unknown checkstyle issue |
Maybe try to rebase on main |
✔️ Finished component verification: 0 component(s) test failed out of 3 component(s) tested |
No description provided.