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

Update odo debugging. #28419

Merged
merged 1 commit into from
Jan 20, 2021
Merged

Update odo debugging. #28419

merged 1 commit into from
Jan 20, 2021

Conversation

yhontyk
Copy link
Contributor

@yhontyk yhontyk commented Jan 7, 2021

Branch 4.6 and 4.7

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 7, 2021
@openshift-docs-preview-bot

The preview will be available shortly at:

@yhontyk yhontyk changed the title [WIP] Update odo debugging. Update odo debugging. Jan 19, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 19, 2021
@bobfuru bobfuru added this to the Next Release milestone Jan 20, 2021
Copy link
Contributor

@bobfuru bobfuru left a comment

Choose a reason for hiding this comment

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

I have a few updates and suggestions.

@@ -10,11 +10,83 @@ You can debug your application on in `odo` with the `odo debug` command.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you fix the typo above this line? "You can debug your application on in..."

@@ -10,11 +10,83 @@ You can debug your application on in `odo` with the `odo debug` command.

.Procedure

. After an application is deployed, start the port forwarding for your component to debug the application:
. Download the sample application which contains the necessary `debugrun` step within its devfile:
Copy link
Contributor

Choose a reason for hiding this comment

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

s/which/that

ISG:

  • Use that, without a comma, to introduce a restrictive clause.
  • Use which, preceded by a comma, to introduce a nonrestrictive clause.

----
$ odo create nodejs --starter
----
+
Copy link
Contributor

Choose a reason for hiding this comment

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

Especially without any introductory text, these output examples should have a title (.Example output):

In addition, prepend the code block for the output with the title .Example output to make it consistently clear across the docs when this is being represented. A lead-in sentence explaining the example output is optional.

See here for details.

Please use `odo push` command to create the component with source deployed
----

. Push the application with the `--debug` flag which is required for all debugging deployments:
Copy link
Contributor

Choose a reason for hiding this comment

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

s/flag which/flag, which

✓ Changes successfully pushed to component
----
+
NOTE: You can specify a custom debug command by using the `--debug-command="custom-step"` flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the preview build I generated, the NOTE admonitions look fine, but our guidelines are to format as such:

[IMPORTANT]
====
lorem ipsum
====

So it might be worth an update to these instances.


. Attach the debugger bundled in your IDE to the component. Instructions vary depending on your IDE.
. Attach the debugger bundled in your IDE of choice. Instructions vary depending on your IDE, for example link:https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_remote-debugging[VSCode debugging interface].
Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity:

  • s/debugger bundled/debugger that is bundled
  • s/for example/for example:

@bobfuru
Copy link
Contributor

bobfuru commented Jan 20, 2021

The preview will be available shortly at:

The preview build didn't work because it was a single module, so it would be good to include a local build hosted on an internal server. I built one locally to check in this case.

Feedback.
@yhontyk
Copy link
Contributor Author

yhontyk commented Jan 20, 2021

@bobfuru thank you for the review! I've addressed all of the issues.
Apologies for the absence of the preview, I didn't notice. :( To be fair, it worked when I clicked "Back to our site" and it took me to the preview of the main page from which I could access the module itself - https://updateododebugging--ocpdocs.netlify.app/openshift-enterprise/latest/cli_reference/developer_cli_odo/creating_and_deploying_applications_with_odo/debugging-applications-in-odo.html
Could this be a problem of just the URL generated by the bot? In any case, I'll make sure to check the build next time.

@bobfuru
Copy link
Contributor

bobfuru commented Jan 20, 2021

@bobfuru thank you for the review! I've addressed all of the issues.
Apologies for the absence of the preview, I didn't notice. :( To be fair, it worked when I clicked "Back to our site" and it took me to the preview of the main page from which I could access the module itself - https://updateododebugging--ocpdocs.netlify.app/openshift-enterprise/latest/cli_reference/developer_cli_odo/creating_and_deploying_applications_with_odo/debugging-applications-in-odo.html
Could this be a problem of just the URL generated by the bot? In any case, I'll make sure to check the build next time.

Interesting, the link you shared here works for me as well. I should have tried the "back to our site" link before. 😁 I thought we had a limitation with netlify that it wouldn't allow preview builds for single-file modules because they lived outside the context of an assembly. But that seems to not quite be the case ¯_(ツ)_/¯ @vikram-redhat WDYT?

Changes all LGTM, thanks for the updates. I'll merge/CP to 4.6, 4.7.

@bobfuru bobfuru added the peer-review-done Signifies that the peer review team has reviewed this PR label Jan 20, 2021
@bobfuru bobfuru merged commit 054dc7c into openshift:master Jan 20, 2021
@bobfuru
Copy link
Contributor

bobfuru commented Jan 20, 2021

/cherrypick enterprise-4.6

@bobfuru
Copy link
Contributor

bobfuru commented Jan 20, 2021

/cherrypick enterprise-4.7

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Jan 20, 2021

@bobfuru: new pull request created: #28726

In response to this:

/cherrypick enterprise-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Jan 20, 2021

@bobfuru: new pull request created: #28727

In response to this:

/cherrypick enterprise-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.6 branch/enterprise-4.7 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants