Skip to content

bazel: add .bazelrc and use Bazel's import instead of symlinking.#66

Merged
htuch merged 2 commits into
envoyproxy:masterfrom
PiotrSikora:bazelrc
Oct 19, 2018
Merged

bazel: add .bazelrc and use Bazel's import instead of symlinking.#66
htuch merged 2 commits into
envoyproxy:masterfrom
PiotrSikora:bazelrc

Conversation

@PiotrSikora
Copy link
Copy Markdown
Contributor

This is future-proofing, since %workspace%/tools/bazel.rc won't be
automatically processed starting with Bazel 0.19 (October 2018).

Signed-off-by: Piotr Sikora piotrsikora@google.com

Comment thread .bazelrc Outdated
@@ -0,0 +1 @@
envoy/.bazelrc No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you mean import envoy/.bazelrc ?

Copy link
Copy Markdown
Contributor Author

@PiotrSikora PiotrSikora Oct 17, 2018

Choose a reason for hiding this comment

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

import envoy/.bazelrc would also work, but this symbolic link pointing to .bazelrc inside envoy submodule (which isn't obvious when reviewing on GitHub) is used for consistency with how it's done for tools/bazel.rc.

I don't have any preference, really, @htuch?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, I wasn't aware of this is a symbolic link, it's GItHub's failure ;) either way works for me too.

This is future-proofing, since %workspace%/tools/bazel.rc won't be
automatically processed starting with Bazel 0.19 (October 2018).

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora changed the title bazel: add .bazelrc. bazel: add .bazelrc and use Bazel's import instead of symlinking. Oct 19, 2018
@PiotrSikora
Copy link
Copy Markdown
Contributor Author

@htuch PTAL.

Copy link
Copy Markdown
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.

For tools/bazel.rc, you can nuke this one, since we are using this repo to demonstrate best practice and nobody should be depending on it.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Copy Markdown
Contributor Author

@htuch done.

@htuch htuch merged commit c1d8c33 into envoyproxy:master Oct 19, 2018
@PiotrSikora
Copy link
Copy Markdown
Contributor Author

Oops, nuking that file broke things completely, since envoy/.bazelrc imports %workspace/tools/bazel.rc which doesn't exist now.

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.

3 participants