Skip to content

doc: improve build guide doc#15764

Merged
mattklein123 merged 3 commits intoenvoyproxy:mainfrom
qinggniq:main
Mar 31, 2021
Merged

doc: improve build guide doc#15764
mattklein123 merged 3 commits intoenvoyproxy:mainfrom
qinggniq:main

Conversation

@qinggniq
Copy link
Copy Markdown
Contributor

Signed-off-by: qinggniq livewithblank@gmail.com

Due to recent update of python package , now only python version >3.8.0 can use ./tools/vscode/refresh_compdb.sh, we should let developers know.

related discussion
https://envoyproxy.slack.com/archives/C78HA81DH/p1617087308144200

Commit Message: improve build guide doc
Additional Description:
Risk Level: Low
Testing: No
Docs Changes: Yes
Release Notes: No
Platform Specific Features: No
[Optional Runtime guard:]
[Optional Fixes #Issue] #15754
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: qinggniq <livewithblank@gmail.com>
@qinggniq
Copy link
Copy Markdown
Contributor Author

@phlax

@phlax phlax self-assigned this Mar 30, 2021
Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

thanks @qinggniq - ive suggested a small change to hopefully make it clearer

bazel/README.md Outdated
[clangd](https://clangd.llvm.org/) with supported editors.

For example, use following command to prepare a compilation database:
This requires Python 3.8.0+, download from [here](https://www.python.org/downloads/) if you do not have one. Then, use following command to prepare a compilation database:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can i suggest the follow modification:

This requires Python 3.8.0+, download from [here](https://www.python.org/downloads/) if you do no have it installed already.

Use following command to prepare a compilation database:

phlax
phlax previously approved these changes Mar 30, 2021
Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @qinggniq

@phlax
Copy link
Copy Markdown
Member

phlax commented Mar 30, 2021

Signed-off-by: qinggniq <livewithblank@outlook.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks just some small comments.

/wait

bazel/README.md Outdated
[clangd](https://clangd.llvm.org/) with supported editors.

For example, use following command to prepare a compilation database:
This requires Python 3.8.0+, download from [here](https://www.python.org/downloads/) if you do no have it installed already.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
This requires Python 3.8.0+, download from [here](https://www.python.org/downloads/) if you do no have it installed already.
This requires Python 3.8.0+, download from [here](https://www.python.org/downloads/) if you do not have it installed already.

bazel/README.md Outdated
For example, use following command to prepare a compilation database:
This requires Python 3.8.0+, download from [here](https://www.python.org/downloads/) if you do no have it installed already.

Use following command to prepare a compilation database:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Use following command to prepare a compilation database:
Use the following command to prepare a compilation database:

@phlax
Copy link
Copy Markdown
Member

phlax commented Mar 30, 2021

apologies for my typos

Signed-off-by: qinggniq <livewithblank@gmail.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

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.

3 participants