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

Use rules_jvm_external to fetch turbine #19472

Closed
wants to merge 1 commit into from
Closed

Use rules_jvm_external to fetch turbine #19472

wants to merge 1 commit into from

Conversation

cushon
Copy link
Contributor

@cushon cushon commented Sep 9, 2023

Instead of checking in a precompiled jar, following the example of #17112

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Sep 9, 2023
@sgowroji sgowroji added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Sep 11, 2023
meteorcloudy
meteorcloudy approved these changes Sep 11, 2023
View reviewed changes
Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@meteorcloudy meteorcloudy added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 11, 2023
@meteorcloudy
Copy link
Member

@iancha1992 Can you first import the non-third party change, then the third party change?

@Pavank1992
Copy link
Contributor

Hi @cushon, Could you please help us in resolving the conflict?

@cushon
Use rules_jvm_external to fetch turbine
aaeeabc 
Instead of checking in a precompiled jar, following the example of
bazelbuild#17112
@cushon
Copy link
Contributor Author

cushon commented Sep 11, 2023

Hi @cushon, Could you please help us in resolving the conflict?

Done

@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 13, 2023
@meteorcloudy
Copy link
Member

Reopen until the third party change is also merged.

@meteorcloudy meteorcloudy reopened this Sep 13, 2023
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer and removed team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. labels Sep 13, 2023
copybara-service bot pushed a commit that referenced this pull request Sep 13, 2023
@cushon @Pavank1992
Use rules_jvm_external to fetch turbine
1200ad2 
Instead of checking in a precompiled jar, following the example of
#17112

Partial commit for third_party/*, see #19472.

Signed-off-by: Pavan Singh <pavanksingh@google.com>
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 13, 2023
copybara-service bot pushed a commit that referenced this pull request Sep 13, 2023
@meteorcloudy
Fixes turbine target
ea019d4 
Fix merging of #19472

RELNOTES: None
PiperOrigin-RevId: 565033337
Change-Id: Ia4020e87e5cf4f9846b801a2203f4931f1ce5205
@fmeum
Copy link
Collaborator

fmeum commented Sep 13, 2023

The third_party change wasn't merged correctly and was reverted as part of the following commit: 2482322

This currently breaks CI on master.

@meteorcloudy
Copy link
Member

meteorcloudy commented Sep 13, 2023 •
edited
Loading

Fixed by ea019d4, sorry about the breakage, please rebase.

copybara-service bot pushed a commit that referenced this pull request Sep 13, 2023
@rickeylev @iancha1992
Load Python rules in integration tests instead of using builtins (#2)
a2f5e82 
This is to facilitate moving the rule implementation out of Bazel itself and into rules_python. This also makes the tests pass when an allowlist is used to enforce that the Python rules are being loaded through rules_python instead of used from builtins.

This the same change as before, but with some internal fixes applied.

PiperOrigin-RevId: 565113515
Change-Id: I999dbfaace8cce2168e718093fe418d548ab1b73

Partial commit for third_party/*, see #19472.

Signed-off-by: Hee Cha <heec@google.com>
@iancha1992 iancha1992 reopened this Sep 13, 2023
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Sep 13, 2023
@iancha1992 iancha1992 added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 13, 2023
@meteorcloudy
Copy link
Member

This is now merged and the breakage has been fixed.

@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meteorcloudy meteorcloudy meteorcloudy approved these changes

Assignees
No one assigned
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@cushon @meteorcloudy @Pavank1992 @fmeum @sgowroji @iancha1992