-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 code format and missing dep on "writing a dev service" page #45703
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
🎊 PR Preview 9104473 has been successfully built and deployed to https://quarkus-pr-main-45703-preview.surge.sh/version/main/guides/
|
This comment has been minimized.
This comment has been minimized.
6102ddc
to
b455c92
Compare
* Fix code formating in code blocks * Add a section about the required dependency for writing a dev service
b455c92
to
e8728fe
Compare
Status for workflow
|
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.
Thanks!
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.
Thanks for this. I added a few suggestions. The first one for sure requires a fix.
@@ -5,118 +5,149 @@ https://github.com/quarkusio/quarkus/tree/main/docs/src/main/asciidoc | |||
//// | |||
[id="extension-writing-dev-service"] | |||
= Writing a Dev Service | |||
|
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.
Please don't do that, I know it's not pretty but you can't have nice new lines or these attributes won't be considered document attributes.
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.
If your extension provides APIs for connecting to an external service, it's a good idea to provide a xref:dev-services.adoc[Dev Service] implementation. | ||
If your extension provides APIs for connecting to an external service, it's a good idea to provide a dev service implementation. | ||
|
||
First, you must add the following dependency to your build file : |
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.
I think we need to be more precise here. It has to be added to the build part of the deployment
module.
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, you must add the following dependency to your build file : | |
First, you must add the following dependency to the build file, in your xref:writing-extensions.adoc#project-setup[deployement] module : |
@BuildStep(onlyIfNot = IsNormal.class, onlyIf = GlobalDevServicesConfig.Enabled.class) | ||
public DevServicesResultBuildItem createContainer() { | ||
DockerImageName dockerImageName = DockerImageName.parse("hello-world"); | ||
GenericContainer container = new GenericContainer<>(dockerImageName) | ||
.withExposedPorts(SERVICE_PORT, OTHER_SERVICE_PORT) | ||
.waitingFor(Wait.forLogMessage(".*" + "Started" + ".*", 1)) | ||
.withReuse(true); |
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.
I think it was done this way to be clear it was a method of a class.
I'm not sure what's best really so I'm not asking you to change it, just stating the why.
.waitingFor(Wait.forLogMessage(".*" + "Started" + ".*", 1)) | ||
.waitingFor(Wait.forLogMessage(".*Started.*", 1)) |
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.
I wonder if we should cite the whole constructor call instead so that's it's extra clear? With this on a new line. WDYT?
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.
What about just reusing the container
variable we defined earlier ?
Something like container.waitingFor(Wait.forLogMessage(".*Started.*", 1));
for instance ?
I would also change the withReuse()
example accordingly.
.withReuse(true) | ||
.withReuse(true) |
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.
Here I would also add a bit more context.
Any comment is welcome 😃