Skip to content

io_error: cleanup TODO#17060

Merged
alyssawilk merged 1 commit intoenvoyproxy:mainfrom
daixiang0:rc
Jul 26, 2021
Merged

io_error: cleanup TODO#17060
alyssawilk merged 1 commit intoenvoyproxy:mainfrom
daixiang0:rc

Conversation

@daixiang0
Copy link
Copy Markdown
Member

@daixiang0 daixiang0 commented Jun 21, 2021

Signed-off-by: Long Dai long0dai@foxmail.com

Commit Message:

Rename with a more meaningful name as TODO comments.

Additional Description: Low
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@alyssawilk alyssawilk self-assigned this Jun 21, 2021
alyssawilk
alyssawilk reviewed Jun 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk 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 the clean up! Just one minor nit and CI checks and you're good to go!

@daixiang0 daixiang0 marked this pull request as draft June 22, 2021 08:13
@daixiang0
Copy link
Copy Markdown
Member Author

daixiang0 commented Jun 23, 2021

sorry for force push after rebase main branch.

@daixiang0 daixiang0 changed the title io_error: cleanup TODO [WIP] io_error: cleanup TODO Jun 23, 2021
@alyssawilk
Copy link
Copy Markdown
Contributor

alyssawilk commented Jun 23, 2021

Just to be aware, we've been making some improvements to Envoy tooling for more prompt reviews, but draft PRs are not included in the tooling. I'm updating the contribution guidelines to make that clear (https://github.com/envoyproxy/envoy/pull/17063/files) , but as this PR predates the repokitteh warning I just want to call out that draft PRs are likely not going to get fast turnaround. Please consider marking this as ready for review if it stalls, and you want the assignee to take a look!

@daixiang0
Copy link
Copy Markdown
Member Author

daixiang0 commented Jun 24, 2021

@alyssawilk thanks for great work, I would to make it ready for review soon.

@daixiang0
Copy link
Copy Markdown
Member Author

daixiang0 commented Jun 24, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Jun 24, 2021

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17060 (comment) was created by @daixiang0.

see: more, trace.

@daixiang0
Copy link
Copy Markdown
Member Author

daixiang0 commented Jun 25, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Jun 25, 2021

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17060 (comment) was created by @daixiang0.

see: more, trace.

@daixiang0 daixiang0 force-pushed the rc branch 3 times, most recently from c6e4e9e to 1ca7202 Compare July 20, 2021 08:48
@daixiang0 daixiang0 force-pushed the rc branch 2 times, most recently from 508a983 to 96b315a Compare July 21, 2021 06:46
@daixiang0
io_error: cleanup TODO
dad5b5f 
Signed-off-by: Long Dai <long0dai@foxmail.com>
alyssawilk
alyssawilk approved these changes Jul 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk 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 the clean up!

@alyssawilk
Copy link
Copy Markdown
Contributor

alyssawilk commented Jul 21, 2021

Happy to merge once this is marked as not draft :-)
/wait

@daixiang0 daixiang0 marked this pull request as ready for review July 22, 2021 23:39
@daixiang0
Copy link
Copy Markdown
Member Author

daixiang0 commented Jul 22, 2021

@alyssawilk done.

@daixiang0 daixiang0 changed the title [WIP] io_error: cleanup TODO io_error: cleanup TODO Jul 22, 2021
@alyssawilk alyssawilk merged commit 878393e into envoyproxy:main Jul 26, 2021
@daixiang0 daixiang0 deleted the rc branch July 27, 2021 01:35
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
@daixiang0
io_error: cleanup TODO by renaming member variable (envoyproxy#17060)
da3b670 
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Long Dai <long0dai@foxmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antoniovicente antoniovicente Awaiting requested review from antoniovicente

@asraa asraa Awaiting requested review from asraa

@ggreenway ggreenway Awaiting requested review from ggreenway ggreenway is a code owner

@lambdai lambdai Awaiting requested review from lambdai

@lizan lizan Awaiting requested review from lizan

@mattklein123 mattklein123 Awaiting requested review from mattklein123 mattklein123 is a code owner

1 more reviewer

@alyssawilk alyssawilk alyssawilk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alyssawilk alyssawilk

Labels

waiting

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@daixiang0 @alyssawilk