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.

Sign up for GitHub

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

Jump to bottom

pgv: update to add wasm32 enabled re2 #12129

Closed
wants to merge 6 commits into from
Closed

pgv: update to add wasm32 enabled re2 #12129

wants to merge 6 commits into from

Conversation

Shikugawa
Copy link
Member

@Shikugawa Shikugawa commented Jul 16, 2020 •
edited
Loading

Commit Message: reflect. #12100 (comment). To add wasm32 enabled re2.
Additional Description: As a response to the following comment: #12100 (comment)
Risk Level: Low
Testing: Existing tests
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Shikugawa [email protected]

@Shikugawa
pgv: update
47cb993 
Signed-off-by: Shikugawa <[email protected]>
@Shikugawa
Copy link
Member Author

/assign @htuch

htuch
htuch suggested changes Jul 16, 2020
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks. Can you also update check_format to ensure that the consistency of these files is validated so we don't regress in the future in this way?

@Shikugawa
Copy link
Member Author

Thanks. Can you also update check_format to ensure that the consistency of these files is validated so we don't regress in the future in this way?

@htuch Is it enough to take diff between bazel/*.bzl and generated_api_shadow/bazel/*.bzl in check_format?

@htuch
Copy link
Member

htuch commented Jul 16, 2020

@Shikugawa yep!

@Shikugawa Shikugawa mentioned this pull request Jul 17, 2020
Merged
htuch pushed a commit that referenced this pull request Jul 17, 2020
@Shikugawa
format: add api-shadow starlark files checker (#12147)
5d8588a 
#12129 (review), to ensure the consistency between api/bazel/*.bzl and generated_api_shadow/bazel/*.bzl

Risk Level: Low

Signed-off-by: Shikugawa <[email protected]>
@dio
Copy link
Member

dio commented Jul 19, 2020 •
edited
Loading

@Shikugawa could you check CI and expand the commit message please? Thanks!

@Shikugawa
Copy link
Member Author

@dio This is blocked by this... Do you have any idea about this? bufbuild/protoc-gen-validate#360

@Shikugawa Shikugawa changed the title pgv: update pgv: update to add wasm32 enabled re2 Jul 20, 2020
@stale
Copy link

stale bot commented Jul 27, 2020

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 27, 2020
@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Jul 28, 2020
Shikugawa added 2 commits July 28, 2020 23:15
@Shikugawa
Kick CI
16d8e83 
Signed-off-by: Shikugawa <[email protected]>
@Shikugawa
Kick CI
4cdf261 
Signed-off-by: Shikugawa <[email protected]>
@Shikugawa
Copy link
Member Author

@htuch PTAL

@Shikugawa
Kick CI
2383375 
Signed-off-by: Shikugawa <[email protected]>
KBaichoo pushed a commit to KBaichoo/envoy that referenced this pull request Jul 30, 2020
@Shikugawa @KBaichoo
format: add api-shadow starlark files checker (envoyproxy#12147)
d35f688 
envoyproxy#12129 (review), to ensure the consistency between api/bazel/*.bzl and generated_api_shadow/bazel/*.bzl

Risk Level: Low

Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: Kevin Baichoo <[email protected]>
scheler pushed a commit to scheler/envoy that referenced this pull request Aug 4, 2020
@Shikugawa
format: add api-shadow starlark files checker (envoyproxy#12147)
9c5bb3b 
envoyproxy#12129 (review), to ensure the consistency between api/bazel/*.bzl and generated_api_shadow/bazel/*.bzl

Risk Level: Low

Signed-off-by: Shikugawa <[email protected]>
Signed-off-by: scheler <[email protected]>
@stale
Copy link

stale bot commented Aug 8, 2020

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Aug 8, 2020
htuch
htuch reviewed Aug 10, 2020
View reviewed changes
bazel/dependency_imports.bzl
@@ -11,6 +11,28 @@ load("@protodoc_pip3//:requirements.bzl", protodoc_pip_install = "pip_install")
# go version for rules_go
GO_VERSION = "1.14.4"

def envoy_api_dependency_import():
Copy link
Member

Choose a reason for hiding this comment

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

How come this change is made to the Envoy bazel/ rather than api/bazel/? If this is something needed for any use of the APIs, it probably belongs in the latter.

@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Aug 10, 2020
@stale
Copy link

stale bot commented Aug 23, 2020

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Aug 23, 2020
@stale
Copy link

stale bot commented Aug 30, 2020

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot closed this Aug 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@htuch htuch htuch requested changes

Assignees

@htuch htuch

Labels
stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Shikugawa @htuch @dio