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

PR for SRVLOGIC-36: [DOC] Modularize the Creating your first workflow service section. #239

Closed
wants to merge 1 commit into from

Conversation

kaldesai
Copy link
Contributor

JIRA: SRVLOGIC-36

Description: Heena was working on this JIRA and her PR was #PR204. Instead of working on her PR, I created my own. (To avoid any merge conflicts). Please take a look at the doc changes and share your feedback.

Please make sure that your PR meets the following requirements:

  • You have read the contributions doc
  • Pull Request title is properly formatted: KOGITO-XYZ Subject
  • Pull Request title contains the target branch if not targeting main: [0.9.x] KOGITO-XYZ Subject
  • The nav.adoc file has a link to this guide in the proper category
  • The index.adoc file has a card to this guide in the proper category, with a meaningful description
How to set up automatic cherry-pick to another branch?

The cherry-pick action allows to setup of automatic cherry-pick from main to a specific branch.

To allow it, you will need to add the corresponding label with pattern: backport-{RELEASE_BRANCH}.
For example, if a backport to branch 1.26.x is needed, then the label should be backport-1.26.x.

Once the PR is merged, the action will retrieve the commit and cherry-pick it to the desired branch.

NOTE: You can still add label after the merge and it should still be cherry-picked.

@netlify
Copy link

netlify bot commented Oct 18, 2022

Deploy Preview for tangerine-crostata-04a6bf ready!

Name Link
🔨 Latest commit 493c386
🔍 Latest deploy log https://app.netlify.com/sites/tangerine-crostata-04a6bf/deploys/634efacb8c78d000081a9f33
😎 Deploy Preview https://deploy-preview-239--tangerine-crostata-04a6bf.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@radtriste radtriste left a comment

Choose a reason for hiding this comment

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

Please update the link on the index page for the Creating your first workflow service card

Comment on lines +12 to +15
* xref:getting-started/modules/proc-boostrapping-the-project.adoc[Bootstrapping a project]
* xref:getting-started/modules/proc-creating-workflow.adoc[Creating a workflow]
* xref:getting-started/modules/proc-running-application.adoc[Running your workflow application]
* xref:getting-started/modules/proc-testing-application.adoc[Testing your workflow application]
Copy link
Contributor

Choose a reason for hiding this comment

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

Those should be "internal" links corresponding to titles, like for example, for the first link:

#proc-boostrapping-the-project_Serverless%20Workflow

else you get redirected to the module page and have no access to the rest of the main page.

* `Inject Mantra`: Injects a `Mantra` message into the response

.Example Hello World workflow
image::../../getting-started/images/first-workflow/hello-world-workflow.png[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Link is not working

@ricardozanini ricardozanini added this to the DP3 milestone Nov 18, 2022
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

There are a few comments from @radtriste that we need to take a look. Also, the cards page has a broken link to the "Creating your first workflow service" guide.

Copy link
Member

@MarianMacik MarianMacik left a comment

Choose a reason for hiding this comment

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

All the things guys found before me + I am adding a little more.

Comment on lines +32 to +36
* `{kogito_sw_ga}`: Adds support for workflows.
* `quarkus-container-image-jib`: Adds support for Container Image Builds.
* `quarkus-resteasy-jackson`: Adds support for RESTEasy, which is required by the generated REST resources that are used to start the flow process using an HTTP request.
* `quarkus-smallrye-openapi`: Adds support for Swagger documentation when you run the application in development mode.
* `--no-code`: Prevents workflow example code from being generated.
Copy link
Member

Choose a reason for hiding this comment

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

The list here is not correctly rendered now.

Comment on lines +36 to +53
Knative workflow CLI:
+
--
Currently, Knative workflow CLI doesn't support yet running workflows locally. You can either:
.Run your workflow application using Apache Maven
[source,shell]
----
mvn clean quarkus:dev
----
.Run your workflow application using Quarkus CLI
[source,shell]
----
quarkus dev
----
For more information about Knative workflow CLI, see xref:tooling/kn-plugin-workflow-overview.adoc[{context} plug-in for Knative CLI].
Also, to deploy and run your workflow application, see xref:cloud/deploying-on-minikube.adoc[Deploying workflow application on Minikube]
--
====
Copy link
Member

Choose a reason for hiding this comment

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

This part of tabs is not correctly rendered. This was also present before but maybe we can fix it now. The issue is probably a missing second colon, so it should be Knative workflow CLI::

--
====
+
.Example response
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.Example response
.Example output

sounds better as this is not a REST request.

include::proc-testing-application.adoc[leveloffset=+1]


.Additional resources
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.Additional resources
== Additional resources

I think it was better when it was a new section. Now it looks like it is under the testing part.

@@ -0,0 +1,4 @@
.Getting Started
** xref:getting-started/modules/create-your-first-workflow-service.adoc[Creating your first workflow service]
Copy link
Member

Choose a reason for hiding this comment

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

Oh, and one thing, just FYI that now we have the word modules twice in the path, it can cause confusion. But if you are fine with that as a docs team, I am OK with that too.

@ricardozanini ricardozanini modified the milestones: DP3, DP4 Dec 2, 2022
+
.Example response
[source,shell,subs="attributes"]
----
Copy link
Member

Choose a reason for hiding this comment

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

maybe we can shrink these logs and show only what indicates that the service is fully up?
wdyt?

@domhanak
Copy link
Contributor

Closing as stale, please reopen if still relevant.

@domhanak domhanak closed this Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants