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

Followup #278 Assert that quarkus-extension.json exists for each exte… #284

Merged
merged 2 commits into from Oct 21, 2019

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Oct 18, 2019

…nsion

@lburgazzoli
Copy link
Contributor

FWIW the file is auto generated by the quarkus plugin in any case

@ppalaga
Copy link
Contributor Author

ppalaga commented Oct 18, 2019

FWIW the file is auto generated by the quarkus plugin in any case

My understanding is that it is generated in ${project.build.outputDirectory} unless it exists in src/main/resources. But we definitely want a custom file in src/main/resources because of the labels and name. So we should check that we have it, should we not?

@lburgazzoli
Copy link
Contributor

IMHO, the validation is meaningful if it checks that the content has some minimal info, checking only the existence of the file does not add real value

@johnpoth
Copy link
Member

Also all these build checks are a bit painful when developing... Any chance to move them to a Profile ? Similar to what is done in Camel with the sourcecheck profile?

@ppalaga
Copy link
Contributor Author

ppalaga commented Oct 18, 2019

IMHO, the validation is meaningful if it checks that the content has some minimal info, checking only the existence of the file does not add real value

Added some checks in 7203021

Also all these build checks are a bit painful when developing... Any chance to move them to a Profile ? Similar to what is done in Camel with the sourcecheck profile?

I moved it to validate phase so that these issues pop up at the very end. I do not think this should be optional because failing to deliver a valid metadata is a real functional problem.

@asf-ci
Copy link

asf-ci commented Oct 18, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/camel-quarkus-pr/278/

Build result: FAILURE

[...truncated 1.48 MB...] at java.lang.reflect.Method.invoke (Method.java:498) at hudson.maven.Maven3Builder.call (Maven3Builder.java:139) at hudson.maven.Maven3Builder.call (Maven3Builder.java:70) at hudson.remoting.UserRequest.perform (UserRequest.java:212) at hudson.remoting.UserRequest.perform (UserRequest.java:54) at hudson.remoting.Request$2.run (Request.java:369) at hudson.remoting.InterceptingExecutorService$1.call (InterceptingExecutorService.java:72) at java.util.concurrent.FutureTask.run (FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:624) at java.lang.Thread.run (Thread.java:748)[ERROR] [ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :camel-quarkus-integration-test-beanchannel stoppedAdding one-line test results to commit status...Setting status of 03ac64f to FAILURE with url https://builds.apache.org/job/camel-quarkus-pr/278/ and message: 'FAILURE 76 tests run, 2 skipped, 0 failed.'Using context: default

@asf-ci
Copy link

asf-ci commented Oct 18, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/camel-quarkus-pr/279/

Build result: FAILURE

[...truncated 1.48 MB...] at java.lang.reflect.Method.invoke (Method.java:498) at hudson.maven.Maven3Builder.call (Maven3Builder.java:139) at hudson.maven.Maven3Builder.call (Maven3Builder.java:70) at hudson.remoting.UserRequest.perform (UserRequest.java:212) at hudson.remoting.UserRequest.perform (UserRequest.java:54) at hudson.remoting.Request$2.run (Request.java:369) at hudson.remoting.InterceptingExecutorService$1.call (InterceptingExecutorService.java:72) at java.util.concurrent.FutureTask.run (FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:624) at java.lang.Thread.run (Thread.java:748)[ERROR] [ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :camel-quarkus-integration-test-beanchannel stoppedAdding one-line test results to commit status...Setting status of 7203021 to FAILURE with url https://builds.apache.org/job/camel-quarkus-pr/279/ and message: 'FAILURE 76 tests run, 2 skipped, 0 failed.'Using context: default

@lburgazzoli
Copy link
Contributor

@ppalaga can you rebase?

@ppalaga
Copy link
Contributor Author

ppalaga commented Oct 19, 2019

2ce6dc7 rebased

final boolean ancillary = (shortName.startsWith('core') || shortName.endsWith('-common'))
if (!extensionJsonFile.exists()) {
messages.add(shortPath + ' is missing')
} else if (!ancillary) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually not sure we want to ignore all these checks in the ancillary extensions. WDYT @lburgazzoli ? Maybe they should have a name, empty labels (or maybe integration, internal ?) and the guide does not matter?

Copy link
Contributor

Choose a reason for hiding this comment

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

core-* do already have the extension metadata file (well core-cloud has an empty one but that's a mistake) and probably xml-common should now be renamed core-xml so yes, every non support extension should have proper metdata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. One more question: do we still consider the extensions satisfying shortName.startsWith('core') || shortName.endsWith('-common') to be "not end user facing"? - I mean should those maybe always have some sort of internal label that the script should enforce?

Copy link
Contributor

Choose a reason for hiding this comment

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

don't think so, it' perfectly acceptable for people using core alone and internal is a little misleading i.e. right now if you want to have xml support you have to explicitly add xml-common

@asf-ci
Copy link

asf-ci commented Oct 19, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/camel-quarkus-pr/284/

@ppalaga
Copy link
Contributor Author

ppalaga commented Oct 21, 2019

0557eee updated based on the discussion with @lburgazzoli

"name": "Camel Quarkus HTTP Common",
"labels": [
"integration",
"camel"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me add HTTP here

@ppalaga
Copy link
Contributor Author

ppalaga commented Oct 21, 2019

0557eee Added labels to XML and HTML common

@asf-ci
Copy link

asf-ci commented Oct 21, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/camel-quarkus-pr/285/

@asf-ci
Copy link

asf-ci commented Oct 21, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/camel-quarkus-pr/286/

@lburgazzoli lburgazzoli merged commit b00dc4c into apache:master Oct 21, 2019
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.

None yet

4 participants