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

Move LabelConversionContext to a new top-level class. #14815

Closed
wants to merge 2 commits into from

Conversation

katre
Copy link
Member

@katre katre commented Feb 14, 2022

Also add a convenience method to create a label converter for a starlark
thread.

This makes it more easily available for other uses.

Part of Optional Toolchains (#14726).

Also add a convenience method to create a label converter for a starlark
thread.

This makes it more easily available for other uses.

Part of Optional Toolchains (bazelbuild#14726).
this.convertedLabelsInPackage = convertedLabelsInPackage;
}

Label getParent() {
Copy link
Member

Choose a reason for hiding this comment

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

maybe "Base" (as in base label + relative label, cf. path)? and/or add some light javadoc ("parent" by itself is a bit confusing IMO)

(also fix up argument in constructor)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, done.

I also cleaned up the other getter methods on LabelConverter, and fixed some logic that uses the converter in BuildType.

Also cleaned up exposed methods on LabelConverter, and fixed some logic
in BuildType to be clearer.
@bazel-io bazel-io closed this in f0cadba Feb 17, 2022
@katre katre deleted the i14726-base-label-converter branch February 17, 2022 23:40
@ShreeM01 ShreeM01 added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants