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.

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

Fix some tests in preparation for --incompatible_load_proto_rules_from_bzl #9002

Closed
wants to merge 5 commits into from

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Jul 28, 2019

No description provided.

@@ -465,6 +465,22 @@ http_archive(
EOF
}

function add_rules_java_to_workspace() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does rules_java need to be added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rules_java will be a dependency of protobuf after #9001.

Copy link
Contributor

Choose a reason for hiding this comment

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

That raises quite a few questions wrt. dependency management, but all of them are out of scope for this pull request and let's try to fix problems one at a time. Approved!

@lberki
Copy link
Contributor

lberki commented Jul 29, 2019

(It appears that a lot of tests are still broken. Is this ready for review?)

@irengrig irengrig added the team-Rules-Server Issues for serverside rules included with Bazel label Jul 31, 2019
@Yannic Yannic changed the title Fix some native proto tests in preparation for --incompatible_load_proto_rules_from_bzl Fix some tests in preparation for --incompatible_load_proto_rules_from_bzl Aug 1, 2019
Copy link
Contributor Author

@Yannic Yannic left a comment

Choose a reason for hiding this comment

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

This change is ready for review. Thanks!

@@ -465,6 +465,22 @@ http_archive(
EOF
}

function add_rules_java_to_workspace() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

rules_java will be a dependency of protobuf after #9001.

@Yannic Yannic marked this pull request as ready for review August 1, 2019 18:41
@lberki
Copy link
Contributor

lberki commented Aug 6, 2019

FYI: I got into some trouble with keeping our internal integration tests green with this change. Stay tuned!

@lberki
Copy link
Contributor

lberki commented Aug 6, 2019

Update: I'll revert the changes in modify_execution_info_test.sh and merge this change without that. That test contains other affected rules, so it's a problem we have to solve for ourselves, too and I don't want to cook a solution for this particular pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Rules-Server Issues for serverside rules included with Bazel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants