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

Fix #897 List the JVM-only extensions in the docs #898

Merged
merged 2 commits into from Mar 18, 2020

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Mar 17, 2020

Fix #897

@ppalaga
Copy link
Contributor Author

ppalaga commented Mar 17, 2020

d51d679 fixed the failing test

Copy link
Contributor

@jamesnetherton jamesnetherton left a comment

Choose a reason for hiding this comment

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

License checks are failing but otherwise looks good to me.

@ppalaga
Copy link
Contributor Author

ppalaga commented Mar 18, 2020

e4543eb fixed the license header issues

Copy link
Contributor

@lburgazzoli lburgazzoli left a comment

Choose a reason for hiding this comment

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

We should not move the functionalities of the package-maven-plugin outside of the camel-quarkus repository as it will introduces yet another external dependency not under direct control of Apache Camel commiters.

In addition it creates a dependency issue as to update to a new version of Apache Camel would require to update the plugin first.

@ppalaga
Copy link
Contributor Author

ppalaga commented Mar 18, 2020

We should not move the functionalities of the package-maven-plugin outside of the camel-quarkus repository as it will introduces yet another external dependency not under direct control of Apache Camel commiters.

It is true that not every Apache Camel committer has access, but OTOH, there are more than one Apache Camel committer that do have write access there.

The motivation was threefold:

  • The packaging mojos share some code with the create-extension and code formatting mojos.
  • Having a plugin hosted in the same source tree where it is applied, is not ideal from the built performance PoV.
  • When the mojos are hosted in a separate repo, they can be run without building them first, which makes the process easier esp. for the newcomers.

Moving the mojos to a separate repo under the ASF umbrella would mean loosing the flexibility of releasing on demand. My understanding is that the voting process cannot be relaxed even for tools like these, right?

So if you insist on your veto, I am going to move the changes back to camel-quarkus.

In addition it creates a dependency issue as to update to a new version of Apache Camel would require to update the plugin first.

This is not an issue, as long as camel-catalog stays backwards compatible, because it is managed on the Camel Quarkus side https://github.com/apache/camel-quarkus/pull/898/files#diff-600376dffeb79835ede4a0b285078036R239-R243

@lburgazzoli
Copy link
Contributor

lburgazzoli commented Mar 18, 2020

I think it is fine to have the extension generator outside the camel-quarkus repo as it is an optional thing but for the catalog generator and other functionalities provided by the package maven plugin it would be a bad move so IMHO we should be keep it as part of the camel-quarkus repo.

@davsclaus @gnodet @oscerd what don you think ?

@davsclaus
Copy link
Contributor

Yes the quarkus camel catalog should be here, this is what we do in other projects too.

@ppalaga
Copy link
Contributor Author

ppalaga commented Mar 18, 2020

00e3c63 brings the mojos back to the Camel Quarkus source tree

@ppalaga
Copy link
Contributor Author

ppalaga commented Mar 18, 2020

d93f2be should fix the test failure.

@ppalaga
Copy link
Contributor Author

ppalaga commented Mar 18, 2020

0f9eef7 fixed the create mojo config

@ppalaga ppalaga merged commit 107102b into apache:master Mar 18, 2020
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.

List the JVM-only extensions in the docs
5 participants