Skip to content

runtime: updates to alpha checks#18552

Merged
alyssawilk merged 4 commits intoenvoyproxy:mainfrom
alyssawilk:alpha
Oct 13, 2021
Merged

runtime: updates to alpha checks#18552
alyssawilk merged 4 commits intoenvoyproxy:mainfrom
alyssawilk:alpha

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18552 (comment) was created by @alyssawilk.

see: more, trace.

@alyssawilk
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18552 (comment) was created by @alyssawilk.

see: more, trace.

break

if "//" in line:
break
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente Oct 12, 2021

Choose a reason for hiding this comment

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

I think this break will result in the alphabetic check being disabled if any comments are added before a flag in the sorted section for any reason. How about changing this check to "// End alphabetically sorted section." and adding that line before the misplaced flags comment. Alternatively, could we sort the existing flags as a way to simplify the alphabetic flags check?

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente 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 extra annotations to avoid further errors in sort order.

@alyssawilk alyssawilk merged commit 12c96a8 into envoyproxy:main Oct 13, 2021
@alyssawilk alyssawilk deleted the alpha branch February 28, 2022 21:26
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