Skip to content

vscode: add a script to generate debugger config#12917

Merged
lizan merged 5 commits intoenvoyproxy:masterfrom
lizan:vscode_debugger
Sep 21, 2020
Merged

vscode: add a script to generate debugger config#12917
lizan merged 5 commits intoenvoyproxy:masterfrom
lizan:vscode_debugger

Conversation

@lizan
Copy link
Copy Markdown
Member

@lizan lizan commented Sep 1, 2020

Commit Message:
Additional Description:
Risk Level: Low
Testing: local
Docs Changes: Added
Release Notes: N/A

@lizan
vscode: add a script to generate debugger config
235080d 
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan
Copy link
Copy Markdown
Member Author

lizan commented Sep 1, 2020

cc @davinci26

@snowp snowp self-assigned this Sep 2, 2020
snowp
snowp suggested changes Sep 2, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@snowp snowp 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 one suggestion

oschaaf
oschaaf reviewed Sep 2, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@oschaaf oschaaf left a comment

Choose a reason for hiding this comment

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

Nice! Some drive by comments feel free to ignore. Also, out of scope, but I wonder if it is possible and what it would take to pre-generate a populated picker list based on this with all targets

@snowp snowp added the waiting label Sep 3, 2020
@lizan
review
739698f 
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan
Copy link
Copy Markdown
Member Author

lizan commented Sep 4, 2020

Also, out of scope, but I wonder if it is possible and what it would take to pre-generate a populated picker list based on this with all targets

That probably need to be a VS Code extension...

@lizan
fix
b6a4e2d 
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@wrowe
Copy link
Copy Markdown
Contributor

wrowe commented Sep 8, 2020

Some of the folks involved with @envoyproxy/windows-dev or @rmiller14 may be knowledgeable in this subject.

snowp
snowp suggested changes Sep 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

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

Just one nit, otherwise LGTM

tools/vscode/generate_debug_config.py Outdated


def getExecutionRoot(workspace):
# If compilation database exists, use its execution root
Copy link
Copy Markdown
Contributor

@snowp snowp Sep 13, 2020

Choose a reason for hiding this comment

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

Explaining why the comp db execution root is preferred might be nice

snowp
snowp approved these changes Sep 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

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

Thanks!

@lizan
Copy link
Copy Markdown
Member Author

lizan commented Sep 21, 2020

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Sep 21, 2020

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #12917 (comment) was created by @lizan.

see: more, trace.

@lizan lizan merged commit 9a4f6f6 into envoyproxy:master Sep 21, 2020
@lizan lizan deleted the vscode_debugger branch September 21, 2020 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@oschaaf oschaaf oschaaf left review comments

@snowp snowp snowp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@snowp snowp

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lizan @wrowe @snowp @oschaaf