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

bazel: Move third party repositories to c-deps/REPOSITORIES.bzl #56525

Merged
merged 1 commit into from
Nov 12, 2020
Merged

bazel: Move third party repositories to c-deps/REPOSITORIES.bzl #56525

merged 1 commit into from
Nov 12, 2020

Conversation

alan-mas
Copy link
Contributor

@alan-mas alan-mas commented Nov 10, 2020 •
edited
Loading

bazel: Move third party repositories to c-deps/REPOSITORIES.bzl

This is one of the Bazel re-factoring that we are working on
and it is about to move third party repositories out of root WORKSPACE.
fixes #56053

Best practices is to separate external dependencies and it also
hides the repo WORKSPACE from being used by other directories.

We are creating a new .bzl file inside c-deps with all the external dependencies
and then load then inside our root WORKSPACE

Release note: None

@blathers-crl
Copy link

blathers-crl bot commented Nov 10, 2020

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I have added a few people who may be able to assist in reviewing:

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Nov 10, 2020
@blathers-crl blathers-crl bot requested a review from jlinder November 10, 2020 21:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@blathers-crl
Copy link

blathers-crl bot commented Nov 10, 2020

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@jlinder jlinder requested a review from a team November 11, 2020 17:24
otan
otan reviewed Nov 12, 2020
View reviewed changes
Copy link
Contributor

@otan otan left a comment

Choose a reason for hiding this comment

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

LGTM after nits

WORKSPACE Outdated Show resolved Hide resolved
c-deps/REPOSITORIES.bzl Outdated Show resolved Hide resolved
@alan-mas
bazel: Move third party repositories to c-deps/REPOSITORIES.bzl
4936cfe 
This is one of the Bazel re-factoring that we are working on
and it is about to move third party repositories out of root WORKSPACE cockroachdb#56053

Best practices is to separate external dependencies and it also
hides the repo WORKSPACE from beign used by other directories.

We are creating a new .bzl file inside c-deps with all the external dependencies
and then load then inside our root WORKSPACE

Release note: None
@blathers-crl blathers-crl bot requested a review from otan November 12, 2020 15:43
otan
otan approved these changes Nov 12, 2020
View reviewed changes
Copy link
Contributor

@otan otan left a comment

Choose a reason for hiding this comment

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

bors r+

@alan-mas
Copy link
Contributor Author

alan-mas commented Nov 12, 2020 •
edited
Loading

@otan bors merge conflicts, should I need to do a rebase from upstream master?

I am not able to see a log or something about the details. It only says "Permission denied"

@otan
Copy link
Contributor

otan commented Nov 12, 2020

some other PR in the queue seems to merge conflict with this one (https://bors.crdb.io/repositories/56/log#batch-7255)
seems like it hasn't decided which one is the issue yet. if bors fails loudly you'll need to rebase, otherwise it will be retried again.

@alan-mas
Copy link
Contributor Author

Cool, then I will wait for bors to decide.

@craig
Copy link
Contributor

craig bot commented Nov 12, 2020

This PR was included in a batch that was canceled, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Nov 12, 2020

Build succeeded:

@craig craig bot merged commit a5e67b6 into cockroachdb:master Nov 12, 2020
@alan-mas alan-mas deleted the 56053-bzl-rule branch November 12, 2020 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@otan otan otan approved these changes

@jlinder jlinder Awaiting requested review from jlinder

Assignees
No one assigned
Labels
O-community Originated from the community X-blathers-triaged blathers was able to find an owner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

build: Move the third party repositories to a c-deps/repositories.bzl rule
3 participants
@alan-mas @cockroach-teamcity @otan