-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Rename UnloadedToolchainContextKey to ToolchainContextKey. #11336
Conversation
Cleanup leading to toolchain transitions, bazelbuild#10523.
@@ -485,11 +485,11 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc | |||
configuration.getFragmentsMap().keySet(), | |||
BuildOptions.diffForReconstruction(defaultBuildOptions, toolchainOptions)); | |||
|
|||
Map<String, UnloadedToolchainContextKey> unloadedToolchainContextKeys = new HashMap<>(); | |||
Map<String, ToolchainContextKey> unloadedToolchainContextKeys = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these relevant var names be changed to, e.g., toolchainContextKeys? Does UnloadedToolchainContext still exist? Sorry if I'm missing context!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UnloadedToolchainContext still exists, but yes, these should be renamed.
@@ -501,19 +501,19 @@ public RuleContext getRuleContextForTesting( | |||
SkyFunctionEnvironmentForTesting skyfunctionEnvironment = | |||
skyframeExecutor.getSkyFunctionEnvironmentForTesting(eventHandler); | |||
|
|||
Map<String, UnloadedToolchainContextKey> unloadedToolchainContextKeys = new HashMap<>(); | |||
Map<String, ToolchainContextKey> unloadedToolchainContextKeys = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same renaming comment here and below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Cleanup leading to toolchain transitions, #10523.