tools: ignore ./bazel-* in check_repositories.sh#7239
Closed
rebello95 wants to merge 1 commit intoenvoyproxy:masterfrom
Closed
tools: ignore ./bazel-* in check_repositories.sh#7239rebello95 wants to merge 1 commit intoenvoyproxy:masterfrom
./bazel-* in check_repositories.sh#7239rebello95 wants to merge 1 commit intoenvoyproxy:masterfrom
Conversation
We currently ignore `./bazel-*` in the `check_format.py` script, but not here. Adding it here as an ignored directory as well to prevent lint failures when using `envoy` as a submodule, such as the following: ``` Checking repositories definitions ./bazel-envoy-mobile/external/bazel_tools/tools/build_defs/repo/git.bzl:68: remote = ctx.attr.remote, ./bazel-envoy-mobile/external/bazel_tools/tools/build_defs/repo/git.bzl:96: actual_commit = ctx.execute([ Using git repositories is not allowed. To ensure that all dependencies can be stored offline in distdir, only HTTP repositories are allowed. ``` Signed-off-by: Michael Rebello <mrebello@lyft.com>
mattklein123
requested changes
Jun 11, 2019
Member
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks for taking this on. Quick question.
/wait
| # Check whether any git repositories are defined. | ||
| # Git repository definition contains `commit` and `remote` fields. | ||
| if grep -nr "commit =\|remote =" --include=*.bzl .; then | ||
| if grep -nr "commit =\|remote =" --include=*.bzl --exclude=./bazel-* .; then |
Member
There was a problem hiding this comment.
Thanks for working on this. From my quick look, I don't think we want to exclude this because this seems to pass OK upstream so we must be pulling in something into our repo which is causing this to fail. I think either we should figure out how to fix our repo or potentially make this exclusion list available via an ENV_VARIABLE that we can set somehow? WDYT?
Contributor
Author
|
Closing in favor of #7245 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We currently ignore
./bazel-*in thecheck_format.pyscript, but not here. Adding it here as an ignored directory as well to prevent lint failures when usingenvoyas a submodule, such as the following:Signed-off-by: Michael Rebello mrebello@lyft.com
Risk Level: Low
Testing: Done locally