Skip to content

Commit

Permalink
Respond to review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
gregestren committed Jul 13, 2020
1 parent 333f11e commit bf60f77
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
11 changes: 6 additions & 5 deletions tools/ctexplain/bazel_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,12 @@ def run_bazel_in_client(args: List[str]) -> Tuple[int, str, str]:
class BazelApi():
"""API that accepts injectable Bazel invocation logic."""

def __init__(self, run_bazel: Callable[List[str]] = run_bazel_in_client):
def __init__(self, run_bazel: Callable[
List[str], Tuple[int, str, str]] = run_bazel_in_client):
self.run_bazel = run_bazel

def cquery(self, args: List[str]) -> Tuple[
bool, str, Tuple[ConfiguredTarget]]:
bool, str, Tuple[ConfiguredTarget, ...]]:
"""Calls cquery with the given arguments.
Args:
Expand Down Expand Up @@ -135,16 +136,16 @@ def _parse_cquery_result_line(line: str) -> ConfiguredTarget:
Returns:
Corresponding ConfiguredTarget if the line matches else None.
"""
tokens = line.split()
tokens = line.split(maxsplit=2)
label = tokens[0]
if tokens[1][0] != "(" or tokens[1][-1] != ")":
raise ValueError(f"{tokens[1]} in {line} not surrounded by parentheses")
config_hash = tokens[1][1:-1]
if config_hash == "null":
fragments = ()
else:
if tokens[2][0] != "[" or tokens[-1][-1] != "]":
raise ValueError(f"{tokens[2:]} in {line} not surrounded by [] brackets")
if tokens[2][0] != "[" or tokens[2][-1] != "]":
raise ValueError(f"{tokens[2]} in {line} not surrounded by [] brackets")
# The fragments list looks like '[Fragment1, Fragment2, ...]'. Split the
# whole line on ' [' to get just this list, then remove the final ']', then
# split again on ', ' to convert it to a structured tuple.
Expand Down
10 changes: 5 additions & 5 deletions tools/ctexplain/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.
"""The core data types ctexplain manipulates."""

from typing import Dict
from typing import Mapping
from typing import Optional
from typing import Tuple
from dataclasses import dataclass
Expand All @@ -27,13 +27,13 @@ class Configuration():
# BuildConfiguration.Fragments in this configuration, as base names without
# packages. For example: ["PlatformConfiguration", ...].
fragments: Tuple[str, ...]
# Dict of FragmentOptions to option key/value pairs. For example:
# Mapping of FragmentOptions to option key/value pairs. For example:
# {"CoreOptions": {"action_env": "[]", "cpu": "x86", ...}, ...}.
#
# Option values are stored as strings of whatever "bazel config" outputs.
#
# Note that Fragment and FragmentOptions aren't the same thing.
options: Dict[str, Dict[str, str]]
options: Mapping[str, Mapping[str, str]]

def __hash__(self):
return self._hash_value()
Expand Down Expand Up @@ -80,7 +80,7 @@ class HostConfiguration(Configuration):
"""
# We don't currently read the host config's fragments or option values.
fragments: Tuple[str, ...] = ()
options: Dict[str, Dict[str, str]] = field(default_factory=lambda: {})
options: Mapping[str, Mapping[str, str]] = field(default_factory=lambda: {})


@dataclass(frozen=True)
Expand All @@ -90,4 +90,4 @@ class NullConfiguration(Configuration):
By definition this has no fragments or options.
"""
fragments: Tuple[str, ...] = ()
options: Dict[str, Dict[str, str]] = field(default_factory=lambda: {})
options: Mapping[str, Mapping[str, str]] = field(default_factory=lambda: {})

0 comments on commit bf60f77

Please sign in to comment.