Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

Propagate implicitly added attributes/arguments to instantiated rules #137

Merged
merged 1 commit into from
Jun 18, 2018

Conversation

ash2k
Copy link

@ash2k ash2k commented May 30, 2018

Fixes #123.

@ash2k ash2k force-pushed the propagate-tags branch 2 times, most recently from 6bc67c1 to d0c6f6c Compare May 30, 2018 12:17
@BenTheElder
Copy link

ping @mattmoor

@ixdy
Copy link

ixdy commented Jun 8, 2018

can you add visibility too, as we just discussed on slack?

k8s/object.bzl Outdated
@@ -399,24 +399,26 @@ def k8s_object(name, **kwargs):
kwargs["image_targets"] = _deduplicate(kwargs.get("images", {}).values())
kwargs["image_target_strings"] = _deduplicate(kwargs.get("images", {}).values())

tags = kwargs.get("tags", None)
Copy link

@ixdy ixdy Jun 8, 2018

Choose a reason for hiding this comment

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

From https://docs.bazel.build/versions/master/skylark/lib/globals.html#rule,

Attributes visibility, deprecation, tags, testonly, and features are implicitly added and cannot be overridden.

Maybe we should have an implicit_attrs dict listing all of these attrs, and pass that as kwargs to the instantiated rules?

@ash2k
Copy link
Author

ash2k commented Jun 8, 2018

I've made the requested changes but I don't know how to test it and I also don't have much time for it, sorry.

@ash2k
Copy link
Author

ash2k commented Jun 9, 2018

The build is stuck

Copy link

@ixdy ixdy left a comment

Choose a reason for hiding this comment

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

thanks!

@mattmoor
Copy link
Contributor

cc @erain this PR would be good low-risk practice for how we test PRs in this repo (b/c secrets are hard). This is going to be easiest if I walk you through it end-to-end over GVC.

@ash2k ash2k changed the title Propagate tags to instantiated rules Propagate implicitly added attributes/arguments to instantiated rules Jun 10, 2018
@clanstyles
Copy link

What's blocking the merge?

@mattmoor mattmoor changed the base branch from master to prop-tags June 18, 2018 19:16
@mattmoor mattmoor merged commit c97f505 into bazelbuild:prop-tags Jun 18, 2018
@erain
Copy link
Contributor

erain commented Jun 18, 2018

Sorry @mattmoor I missed this PR and the conversation. I will take the next chance to learn from the PR tests.

@fejta
Copy link
Contributor

fejta commented Jun 27, 2018

@ash2k any chance you can head over to #143 and sign the CLA so we can merge this into master?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants