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

Stop using //third_party/protobuf/3.6.1 in //src #10648

Closed
wants to merge 1 commit into from
Closed

Stop using //third_party/protobuf/3.6.1 in //src #10648

wants to merge 1 commit into from

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Jan 24, 2020 •
edited
Loading

No description provided.

@Yannic Yannic mentioned this pull request Jan 24, 2020
Closed
@Yannic Yannic changed the title Make protobuf repo external Reference @com_google_protobuf//:* in proto_alias Jan 24, 2020
@irengrig irengrig added the team-Rules-Server Issues for serverside rules included with Bazel label Feb 3, 2020
lberki
lberki reviewed Feb 4, 2020
View reviewed changes
Copy link
Contributor

@lberki lberki left a comment

Choose a reason for hiding this comment

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

I like this change! We should totally not vendor so many things... well, we should totally not have so many dependencies, but protobuf is a must.

Unfortunately, this must be split between parts within third_party and parts outside it because we can't submit this change atomically. @philwo maybe has some helpful suggestions?

@philwo
Copy link
Member

philwo commented Feb 4, 2020

If this entire PR passes CI (which seems fine), I can perform a so called manual atomic override merge. It’s not without risks... but nobody said the job would be easy, right.

@Yannic It’s still marked as draft though - is this ready to be merged?

@Yannic
Copy link
Contributor Author

Yannic commented Feb 4, 2020

@philwo It's marked as draft because it contains commits from #10638 (which adds the logic to include pre-built jars in the bootstrap archive).

Maybe let's start by merging #10638 and then I'll rebase this one so you don't have to do a manual atomic override merge.

@Yannic Yannic force-pushed the make_protobuf_repo_external branch 2 times, most recently from 5414fed to c16bc0f Compare February 5, 2020 11:59
@Yannic Yannic force-pushed the make_protobuf_repo_external branch from c16bc0f to 94b5de6 Compare February 5, 2020 12:00
@Yannic Yannic changed the title Reference @com_google_protobuf//:* in proto_alias Stop using //third_party/protobuf/3.6.1 in //src Feb 5, 2020
@Yannic Yannic marked this pull request as ready for review February 5, 2020 12:03
@Yannic
Copy link
Contributor Author

Yannic commented Feb 5, 2020

@philwo Thanks for merging the dependencies of this yesterday. I've also split this into a //src change and a //third_party change. ptal.

@bazel-io bazel-io closed this in 60e92e2 Feb 6, 2020
@Yannic Yannic deleted the make_protobuf_repo_external branch March 11, 2020 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lberki lberki lberki left review comments

Assignees

@philwo philwo

@lberki lberki

Labels
cla: yes team-Rules-Server Issues for serverside rules included with Bazel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Yannic @philwo @lberki @googlebot @irengrig