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

Modified docs/development-guide.md to Reorganize prerequisites, add kubectl link, clone repo and accessing frontend #1853

Merged
merged 6 commits into from
Jun 21, 2023

Conversation

taka2noda
Copy link
Contributor

Background

  1. Prerequisites for Options 1 and 2 appear mixed.
  2. A guide for using 'kubectl' locally seems absent.
  3. It seems 'git clone' instructions are missing.
  4. Access steps to the web front are missing.

Fixes

Change Summary

  1. Reordered prerequisites for better clarity and indicated items exclusive to either Option 1 or Option 2.
  2. Added a link to kubectl in the prerequisites.
  3. Added instructions to clone the repository.
  4. Added a step in Option 1 to access the web frontend via http://EXTERNAL-IP.

Additional Notes

Testing Procedure

  1. Checked at my own forked repository.
  2. Check to build at my own GCP account by following updated docs.

Related PRs or Issues

1. Reordered prerequisites for better clarity and indicated items exclusive to either Option 1 or Option 2.
2. Added a link to kubectl in the prerequisites.
3. Added instructions to clone the repository.
4. Added a step in Option 1 to access the web frontend via http://EXTERNAL-IP.
@taka2noda taka2noda requested review from a team and yoshi-approver as code owners June 20, 2023 05:06
Copy link
Collaborator

@NimJay NimJay left a comment

Choose a reason for hiding this comment

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

Thanks for these improvements.
I left some comments.
Feel free to ignore my "nitpicks".

@taka2noda taka2noda requested a review from NimJay June 21, 2023 12:07
taka2noda and others added 4 commits June 21, 2023 22:06
Co-authored-by: Nim Jayawardena <[email protected]>
Co-authored-by: Nim Jayawardena <[email protected]>
Co-authored-by: Nim Jayawardena <[email protected]>
1. Remove `Enable GCP APIs for Cloud Monitoring, Tracing, Profiler`
2. Restore `Docker for Desktop` position and remove (for Option 2)
3. Change `Option1` to `Option 1` by following `Option 2`
@taka2noda
Copy link
Contributor Author

Hi @NimJay Thank you for the review.
I have addressed the change requests you provided and committed the changes again.
Could you please confirm these changes?

The details of the changes are as follows.

  1. Remove Enable GCP APIs for Cloud Monitoring, Tracing, Profiler
  2. Restore Docker for Desktop position as top and remove (for Option 2)
  3. Change Option1 to Option 1 by following Option2 to Option 2

Thank you.

@taka2noda
Copy link
Contributor Author

Hi @NimJay
Also, based on the results of the workflow, it seems that the your CLA is lacking.
https://github.com/GoogleCloudPlatform/microservices-demo/pull/1853/checks?check_run_id=14438448249

Although I don't anticipate any issues, I would appreciate if you could confirm this as well.
Thank you.

Copy link
Collaborator

@NimJay NimJay left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution, @taka2noda. :)
And for addressing all my comments.

Ah, yes, the CLA was failing on my personal email address. I forced it to pass.

Approved. Merging.

@NimJay NimJay merged commit 1ef0a0c into GoogleCloudPlatform:main Jun 21, 2023
@taka2noda taka2noda deleted the update-development-guide branch June 21, 2023 14:39
mrcrgl pushed a commit to fiberfjord/microservices-demo that referenced this pull request Sep 11, 2023
…ubectl link, clone repo and accessing frontend (GoogleCloudPlatform#1853)

* Modified docs/development-guide.md

1. Reordered prerequisites for better clarity and indicated items exclusive to either Option 1 or Option 2.
2. Added a link to kubectl in the prerequisites.
3. Added instructions to clone the repository.
4. Added a step in Option 1 to access the web frontend via http://EXTERNAL-IP.

* Update docs/development-guide.md

Co-authored-by: Nim Jayawardena <[email protected]>

* Update docs/development-guide.md

Co-authored-by: Nim Jayawardena <[email protected]>

* Update docs/development-guide.md

Co-authored-by: Nim Jayawardena <[email protected]>

* Update reffering to the review.

1. Remove `Enable GCP APIs for Cloud Monitoring, Tracing, Profiler`
2. Restore `Docker for Desktop` position and remove (for Option 2)
3. Change `Option1` to `Option 1` by following `Option 2`

* Change Option1 to Option 1 at line8 kubectl...

---------

Co-authored-by: Nim Jayawardena <[email protected]>
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.

2 participants