From 7b7889acdd4fa5d958474c9125822f933ec6837a Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 5 Sep 2018 14:24:44 -0700 Subject: [PATCH 1/3] FIXME -> TODO for all comments --- build-support/bin/ci.sh | 2 +- .../src/python/internal_backend/sitegen/tasks/sitegen.py | 2 +- src/python/pants/backend/native/config/environment.py | 2 +- src/python/pants/backend/native/register.py | 2 +- .../pants/backend/native/subsystems/binaries/llvm.py | 2 +- .../backend/native/subsystems/native_compile_settings.py | 2 +- .../pants/backend/native/subsystems/xcode_cli_tools.py | 2 +- .../pants/backend/native/targets/native_artifact.py | 2 +- .../pants/backend/native/tasks/link_shared_libraries.py | 4 ++-- src/python/pants/backend/native/tasks/native_compile.py | 8 ++++---- .../backend/native/tasks/native_external_library_fetch.py | 2 +- src/python/pants/backend/native/tasks/native_task.py | 2 +- src/python/pants/binaries/binary_util.py | 2 +- src/python/pants/build_graph/target.py | 2 +- src/python/pants/fs/archive.py | 2 +- src/python/pants/goal/products.py | 2 +- src/python/pants/option/scope.py | 2 +- src/rust/engine/src/context.rs | 2 +- src/rust/engine/src/nodes.rs | 2 +- .../pants_test/backend/native/util/platform_utils.py | 2 +- 20 files changed, 24 insertions(+), 24 deletions(-) diff --git a/build-support/bin/ci.sh b/build-support/bin/ci.sh index 2fc765661b1..c5d4fa06599 100755 --- a/build-support/bin/ci.sh +++ b/build-support/bin/ci.sh @@ -133,7 +133,7 @@ if [[ "${python_three:-false}" == "true" ]]; then # TODO(John Sirois): Allow `<4` when the issues with `3.7` are fixed. See: # https://github.com/pantsbuild/pants/issues/6363 export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS='["CPython>=3.4,<3.7"]' - # FIXME: Clear interpreters, otherwise this constraint does not end up applying due to a cache + # TODO: Clear interpreters, otherwise this constraint does not end up applying due to a cache # bug between the `./pants binary` and further runs. ./pants.pex clean-all fi diff --git a/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py b/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py index b4db70fdc6e..4d2825004be 100644 --- a/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py +++ b/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py @@ -47,7 +47,7 @@ def register_options(cls, register): super(SiteGen, cls).register_options(register) register('--config-path', type=list, help='Path to .json file describing site structure.') - # FIXME(cosmicexplorer): requiring these products ensures that the markdown + # TODO(cosmicexplorer): requiring these products ensures that the markdown # and reference tasks run before this one, but we don't use those # products. @classmethod diff --git a/src/python/pants/backend/native/config/environment.py b/src/python/pants/backend/native/config/environment.py index a72c684cd68..d906d54cfb8 100644 --- a/src/python/pants/backend/native/config/environment.py +++ b/src/python/pants/backend/native/config/environment.py @@ -187,7 +187,7 @@ class LLVMCppToolchain(datatype([('cpp_toolchain', CppToolchain)])): pass class GCCCppToolchain(datatype([('cpp_toolchain', CppToolchain)])): pass -# FIXME: make this an @rule, after we can automatically produce LibcDev and other subsystems in the +# TODO: make this an @rule, after we can automatically produce LibcDev and other subsystems in the # v2 engine (see #5788). class HostLibcDev(datatype(['crti_object', 'fingerprint'])): diff --git a/src/python/pants/backend/native/register.py b/src/python/pants/backend/native/register.py index a9a11e7d81b..bca4327d5d6 100644 --- a/src/python/pants/backend/native/register.py +++ b/src/python/pants/backend/native/register.py @@ -35,7 +35,7 @@ def build_file_aliases(): def register_goals(): - # FIXME(#5962): register these under the 'compile' goal when we eliminate the product transitive + # TODO(#5962): register these under the 'compile' goal when we eliminate the product transitive # dependency from export -> compile. task(name='native-third-party-fetch', action=NativeExternalLibraryFetch).install('native-compile') task(name='c-for-ctypes', action=CCompile).install('native-compile') diff --git a/src/python/pants/backend/native/subsystems/binaries/llvm.py b/src/python/pants/backend/native/subsystems/binaries/llvm.py index fe6476d97aa..d10eea1a56c 100644 --- a/src/python/pants/backend/native/subsystems/binaries/llvm.py +++ b/src/python/pants/backend/native/subsystems/binaries/llvm.py @@ -123,7 +123,7 @@ def cpp_compiler(self): extra_args=[]) -# FIXME(#5663): use this over the XCode linker! +# TODO(#5663): use this over the XCode linker! @rule(Linker, [Select(Platform), Select(LLVM)]) def get_lld(platform, llvm): return llvm.linker(platform) diff --git a/src/python/pants/backend/native/subsystems/native_compile_settings.py b/src/python/pants/backend/native/subsystems/native_compile_settings.py index 230d91a10c7..73e459818f7 100644 --- a/src/python/pants/backend/native/subsystems/native_compile_settings.py +++ b/src/python/pants/backend/native/subsystems/native_compile_settings.py @@ -19,7 +19,7 @@ def register_options(cls, register): register('--fatal-warnings', type=bool, default=True, fingerprint=True, advanced=True, help='The default for the "fatal_warnings" argument for targets of this language.') - # FIXME: use some more formal method of mirroring options between a target and a subsystem -- see + # TODO: use some more formal method of mirroring options between a target and a subsystem -- see # pants.backend.jvm.subsystems.dependency_context.DependencyContext#defaulted_property()! def get_subsystem_target_mirrored_field_value(self, field_name, target): """Get the attribute `field_name` from `target` if set, else from this subsystem's options.""" diff --git a/src/python/pants/backend/native/subsystems/xcode_cli_tools.py b/src/python/pants/backend/native/subsystems/xcode_cli_tools.py index 08bad5515cb..80bebb5d4dd 100644 --- a/src/python/pants/backend/native/subsystems/xcode_cli_tools.py +++ b/src/python/pants/backend/native/subsystems/xcode_cli_tools.py @@ -128,7 +128,7 @@ def include_dirs(self): all_inc_dirs = base_inc_dirs for d in base_inc_dirs: - # FIXME: what does this directory do? + # TODO: what does this directory do? secure_inc_dir = os.path.join(d, 'secure') if is_readable_dir(secure_inc_dir): all_inc_dirs.append(secure_inc_dir) diff --git a/src/python/pants/backend/native/targets/native_artifact.py b/src/python/pants/backend/native/targets/native_artifact.py index 54f10a8028c..396f5fdbb0f 100644 --- a/src/python/pants/backend/native/targets/native_artifact.py +++ b/src/python/pants/backend/native/targets/native_artifact.py @@ -28,6 +28,6 @@ def as_shared_lib(self, platform): }) def _compute_fingerprint(self): - # FIXME: can we just use the __hash__ method here somehow? + # TODO: can we just use the __hash__ method here somehow? hasher = sha1(self.lib_name.encode('utf-8')) return hasher.hexdigest() if PY3 else hasher.hexdigest().decode('utf-8') diff --git a/src/python/pants/backend/native/tasks/link_shared_libraries.py b/src/python/pants/backend/native/tasks/link_shared_libraries.py index 965631ff7b0..1440f525a24 100644 --- a/src/python/pants/backend/native/tasks/link_shared_libraries.py +++ b/src/python/pants/backend/native/tasks/link_shared_libraries.py @@ -88,7 +88,7 @@ def linker(self): @memoized_property def platform(self): - # FIXME: convert this to a v2 engine dependency injection. + # TODO: convert this to a v2 engine dependency injection. return Platform.create() def _retrieve_single_product_at_target_base(self, product_mapping, target): @@ -114,7 +114,7 @@ def execute(self): if vt.valid: shared_library = self._retrieve_shared_lib_from_cache(vt) else: - # FIXME: We need to partition links based on proper dependency edges and not + # TODO: We need to partition links based on proper dependency edges and not # perform a link to every native_external_library for all targets in the closure. # https://github.com/pantsbuild/pants/issues/6178 link_request = self._make_link_request( diff --git a/src/python/pants/backend/native/tasks/native_compile.py b/src/python/pants/backend/native/tasks/native_compile.py index 2efe955b0dd..ad57a888548 100644 --- a/src/python/pants/backend/native/tasks/native_compile.py +++ b/src/python/pants/backend/native/tasks/native_compile.py @@ -33,14 +33,14 @@ class NativeCompileRequest(datatype([ ])): pass -# FIXME(#5950): perform all process execution in the v2 engine! +# TODO(#5950): perform all process execution in the v2 engine! class ObjectFiles(datatype(['root_dir', 'filenames'])): def file_paths(self): return [os.path.join(self.root_dir, fname) for fname in self.filenames] -# FIXME: this is a temporary hack -- we could introduce something like a "NativeRequirement" with +# TODO: this is a temporary hack -- we could introduce something like a "NativeRequirement" with # dependencies, header, object file, library name (more?) instead of using multiple products. class NativeTargetDependencies(datatype(['native_deps'])): pass @@ -160,7 +160,7 @@ def get_sources_headers_for_target(self, target): # Unique file names are required because we just dump object files into a single directory, and # the compiler will silently just produce a single object file if provided non-unique filenames. - # FIXME: add some shading to file names so we can remove this check. + # TODO: add some shading to file names so we can remove this check. # NB: It shouldn't matter if header files have the same name, but this will raise an error in # that case as well. We won't need to do any shading of header file names. seen_filenames = defaultdict(list) @@ -178,7 +178,7 @@ def get_sources_headers_for_target(self, target): return [os.path.join(get_buildroot(), rel_root, src) for src in target_relative_sources] - # FIXME(#5951): expand `Executable` to cover argv generation (where an `Executable` is subclassed + # TODO(#5951): expand `Executable` to cover argv generation (where an `Executable` is subclassed # to modify or extend the argument list, as declaratively as possible) to remove # `extra_compile_args(self)`! @abstractmethod diff --git a/src/python/pants/backend/native/tasks/native_external_library_fetch.py b/src/python/pants/backend/native/tasks/native_external_library_fetch.py index b8d228cc856..dc1d5c21d85 100644 --- a/src/python/pants/backend/native/tasks/native_external_library_fetch.py +++ b/src/python/pants/backend/native/tasks/native_external_library_fetch.py @@ -33,7 +33,7 @@ class ConanRequirement(datatype(['pkg_spec'])): } def parse_conan_stdout_for_pkg_sha(self, stdout): - # FIXME: properly regex this method. + # TODO: properly regex this method. # https://github.com/pantsbuild/pants/issues/6168 pkg_line = stdout.split('Packages')[1] collected_matches = [line for line in pkg_line.split() if self.pkg_spec in line] diff --git a/src/python/pants/backend/native/tasks/native_task.py b/src/python/pants/backend/native/tasks/native_task.py index 7f95256f205..9bb8a010b0d 100644 --- a/src/python/pants/backend/native/tasks/native_task.py +++ b/src/python/pants/backend/native/tasks/native_task.py @@ -9,7 +9,7 @@ class NativeTask(Task): - # FIXME(#5869): delete this when we can request Subsystems from options in tasks! + # TODO(#5869): delete this when we can request Subsystems from options in tasks! def _request_single(self, product, subject): # NB: This is not supposed to be exposed to Tasks yet -- see #4769 to track the status of # exposing v2 products in v1 tasks. diff --git a/src/python/pants/binaries/binary_util.py b/src/python/pants/binaries/binary_util.py index 2b962ea456a..5120ac0f356 100644 --- a/src/python/pants/binaries/binary_util.py +++ b/src/python/pants/binaries/binary_util.py @@ -315,7 +315,7 @@ def __init__(self, baseurls, binary_tool_fetcher, path_by_id=None, 'linux': lambda release, machine: ('linux', machine), } - # FIXME(cosmicexplorer): we create a HostPlatform in this class instead of in the constructor + # TODO(cosmicexplorer): we create a HostPlatform in this class instead of in the constructor # because we don't want to fail until a binary is requested. The HostPlatform should be a # parameter that gets lazily resolved by the v2 engine. @memoized_method diff --git a/src/python/pants/build_graph/target.py b/src/python/pants/build_graph/target.py index 1e5044241f5..6006d125db2 100644 --- a/src/python/pants/build_graph/target.py +++ b/src/python/pants/build_graph/target.py @@ -680,7 +680,7 @@ def strict_dependencies(self, dep_context): strict_deps = self._cached_strict_dependencies_map.get(dep_context, None) if strict_deps is None: default_predicate = self._closure_dep_predicate({self}, **dep_context.target_closure_kwargs) - # FIXME(#5977): this branch needs testing! + # TODO(#5977): this branch needs testing! if not default_predicate: def default_predicate(*args, **kwargs): return True diff --git a/src/python/pants/fs/archive.py b/src/python/pants/fs/archive.py index 8fb4dd79dc8..c424f809cbd 100644 --- a/src/python/pants/fs/archive.py +++ b/src/python/pants/fs/archive.py @@ -122,7 +122,7 @@ def _invoke_xz(self, xz_input_file): This allows streaming the decompressed tar archive directly into a tar decompression stream, which is significantly faster in practice than making a temporary file. """ - # FIXME: --threads=0 is supposed to use "the number of processor cores on the machine", but I + # TODO: --threads=0 is supposed to use "the number of processor cores on the machine", but I # see no more than 100% cpu used at any point. This seems like it could be a bug? If performance # is an issue, investigate further. cmd = [self._xz_binary_path, '--decompress', '--stdout', '--keep', '--threads=0', xz_input_file] diff --git a/src/python/pants/goal/products.py b/src/python/pants/goal/products.py index 3d2378e4815..a13a204ea3a 100644 --- a/src/python/pants/goal/products.py +++ b/src/python/pants/goal/products.py @@ -64,7 +64,7 @@ def add_for_targets(self, targets, products): :API: public """ - # FIXME: This is a temporary helper for use until the classpath has been split. + # TODO: This is a temporary helper for use until the classpath has been split. for target in targets: self.add_for_target(target, products) diff --git a/src/python/pants/option/scope.py b/src/python/pants/option/scope.py index c0a4c6f8eb4..f05d9390cf9 100644 --- a/src/python/pants/option/scope.py +++ b/src/python/pants/option/scope.py @@ -11,7 +11,7 @@ GLOBAL_SCOPE_CONFIG_SECTION = 'GLOBAL' -# FIXME: convert this to a datatype? +# TODO: convert this to a datatype? class ScopeInfo(namedtuple('_ScopeInfo', ['scope', 'category', 'optionable_cls'])): """Information about a scope.""" diff --git a/src/rust/engine/src/context.rs b/src/rust/engine/src/context.rs index 96f1c6fbd5f..d69e5c54e70 100644 --- a/src/rust/engine/src/context.rs +++ b/src/rust/engine/src/context.rs @@ -109,7 +109,7 @@ impl Core { fs_pool: fs_pool.clone(), runtime: runtime, store: store, - // FIXME: Errors in initialization should definitely be exposed as python + // TODO: Errors in initialization should definitely be exposed as python // exceptions, rather than as panics. vfs: PosixFS::new(build_root, fs_pool, &ignore_patterns).unwrap_or_else(|e| { panic!("Could not initialize VFS: {:?}", e); diff --git a/src/rust/engine/src/nodes.rs b/src/rust/engine/src/nodes.rs index 3904643277a..5ea728a845f 100644 --- a/src/rust/engine/src/nodes.rs +++ b/src/rust/engine/src/nodes.rs @@ -1037,7 +1037,7 @@ impl Node for NodeKey { fn typstr(tc: &TypeConstraint) -> String { externs::key_to_str(&tc.0) } - // FIXME(cosmicexplorer): these should all be converted to fmt::Debug implementations, and then + // TODO(cosmicexplorer): these should all be converted to fmt::Debug implementations, and then // this method can go away in favor of the auto-derived Debug for this type. match self { &NodeKey::DigestFile(ref s) => format!("DigestFile({:?})", s.0), diff --git a/tests/python/pants_test/backend/native/util/platform_utils.py b/tests/python/pants_test/backend/native/util/platform_utils.py index 11f613648a2..293770a488b 100644 --- a/tests/python/pants_test/backend/native/util/platform_utils.py +++ b/tests/python/pants_test/backend/native/util/platform_utils.py @@ -14,7 +14,7 @@ def platform_specific(normalized_os_name): def decorator(test_fn): def wrapper(self, *args, **kwargs): - # FIXME: This should be drawn from the v2 engine somehow. + # TODO: This should be drawn from the v2 engine somehow. platform = Platform.create() if platform.normalized_os_name == normalized_os_name: From 6de0321f036d602c125b11675e68b74d67b25a59 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 5 Sep 2018 15:50:20 -0700 Subject: [PATCH 2/3] remove (cosmicexplorer) --- .../src/python/internal_backend/sitegen/tasks/sitegen.py | 5 ++--- src/python/pants/binaries/binary_tool.py | 2 +- src/python/pants/binaries/binary_util.py | 6 +++--- src/python/pants/engine/fs.py | 3 +-- src/python/pants/option/parser_hierarchy.py | 3 +-- src/python/pants/pantsd/watchman_launcher.py | 2 +- src/python/pants/util/objects.py | 4 ++-- src/python/pants/util/osutil.py | 2 +- src/rust/engine/fs/src/lib.rs | 4 ++-- src/rust/engine/src/nodes.rs | 4 ++-- tests/python/pants_test/binaries/test_binary_tool.py | 3 +-- tests/python/pants_test/binaries/test_binary_util.py | 2 +- tests/python/pants_test/engine/test_isolated_process.py | 2 -- 13 files changed, 18 insertions(+), 24 deletions(-) diff --git a/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py b/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py index 4d2825004be..7adbc3deb7c 100644 --- a/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py +++ b/pants-plugins/src/python/internal_backend/sitegen/tasks/sitegen.py @@ -47,9 +47,8 @@ def register_options(cls, register): super(SiteGen, cls).register_options(register) register('--config-path', type=list, help='Path to .json file describing site structure.') - # TODO(cosmicexplorer): requiring these products ensures that the markdown - # and reference tasks run before this one, but we don't use those - # products. + # TODO: requiring these products ensures that the markdown and reference tasks run before this + # one, but we don't use those products. @classmethod def prepare(cls, options, round_manager): round_manager.require(MarkdownToHtml.MARKDOWN_HTML_PRODUCT) diff --git a/src/python/pants/binaries/binary_tool.py b/src/python/pants/binaries/binary_tool.py index 27a49ae9fed..08c3bdd23d9 100644 --- a/src/python/pants/binaries/binary_tool.py +++ b/src/python/pants/binaries/binary_tool.py @@ -20,7 +20,7 @@ logger = logging.getLogger(__name__) -# TODO(cosmicexplorer): Add integration tests for this file. +# TODO: Add integration tests for this file. class BinaryToolBase(Subsystem): """Base class for subsytems that configure binary tools. diff --git a/src/python/pants/binaries/binary_util.py b/src/python/pants/binaries/binary_util.py index 5120ac0f356..4affeab1741 100644 --- a/src/python/pants/binaries/binary_util.py +++ b/src/python/pants/binaries/binary_util.py @@ -315,9 +315,9 @@ def __init__(self, baseurls, binary_tool_fetcher, path_by_id=None, 'linux': lambda release, machine: ('linux', machine), } - # TODO(cosmicexplorer): we create a HostPlatform in this class instead of in the constructor - # because we don't want to fail until a binary is requested. The HostPlatform should be a - # parameter that gets lazily resolved by the v2 engine. + # TODO: we create a HostPlatform in this class instead of in the constructor because we don't want + # to fail until a binary is requested. The HostPlatform should be a parameter that gets lazily + # resolved by the v2 engine. @memoized_method def _host_platform(self): uname_result = self._uname_func() diff --git a/src/python/pants/engine/fs.py b/src/python/pants/engine/fs.py index 0a6b6e6d8c4..01398318fe5 100644 --- a/src/python/pants/engine/fs.py +++ b/src/python/pants/engine/fs.py @@ -124,8 +124,7 @@ class DirectoryToMaterialize(datatype([('path', text_type), ('directory_digest', FilesContent = Collection.of(FileContent) -# TODO(cosmicexplorer): don't recreate this in python, get this from -# fs::EMPTY_DIGEST somehow. +# TODO: don't recreate this in python, get this from fs::EMPTY_DIGEST somehow. _EMPTY_FINGERPRINT = 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' diff --git a/src/python/pants/option/parser_hierarchy.py b/src/python/pants/option/parser_hierarchy.py index d2efa903f74..fadf838c7cb 100644 --- a/src/python/pants/option/parser_hierarchy.py +++ b/src/python/pants/option/parser_hierarchy.py @@ -36,8 +36,7 @@ def all_enclosing_scopes(scope, allow_global=True): _validate_full_scope(scope) - # TODO(cosmicexplorer): validate scopes here and/or in `enclosing_scope()` - # instead of assuming correctness. + # TODO: validate scopes here and/or in `enclosing_scope()` instead of assuming correctness. def scope_within_range(tentative_scope): if tentative_scope is None: return False diff --git a/src/python/pants/pantsd/watchman_launcher.py b/src/python/pants/pantsd/watchman_launcher.py index 9095b665621..a8987276b3f 100644 --- a/src/python/pants/pantsd/watchman_launcher.py +++ b/src/python/pants/pantsd/watchman_launcher.py @@ -27,7 +27,7 @@ def create(cls, bootstrap_options): baseurls=bootstrap_options.binaries_baseurls, binary_tool_fetcher=binary_tool_fetcher, path_by_id=bootstrap_options.binaries_path_by_id, - # TODO(cosmicexplorer): do we need to test this? + # TODO: do we need to test this? allow_external_binary_tool_downloads=bootstrap_options.allow_external_binary_tool_downloads) return WatchmanLauncher( diff --git a/src/python/pants/util/objects.py b/src/python/pants/util/objects.py index a55dc5e1360..ea7b60079ba 100644 --- a/src/python/pants/util/objects.py +++ b/src/python/pants/util/objects.py @@ -72,8 +72,8 @@ def __new__(cls, *args, **kwargs): except TypeError as e: raise cls.make_type_error(e) - # TODO(cosmicexplorer): Make this kind of exception pattern (filter for - # errors then display them all at once) more ergonomic. + # TODO: Make this kind of exception pattern (filter for errors then display them all at once) + # more ergonomic. type_failure_msgs = [] for field_name, field_constraint in fields_with_constraints.items(): field_value = getattr(this_object, field_name) diff --git a/src/python/pants/util/osutil.py b/src/python/pants/util/osutil.py index e0a6361c0ac..132ee71c47d 100644 --- a/src/python/pants/util/osutil.py +++ b/src/python/pants/util/osutil.py @@ -52,7 +52,7 @@ def known_os_names(): return reduce(set.union, OS_ALIASES.values()) -# TODO(cosmicexplorer): use this as the default value for the global --binaries-path-by-id option! +# TODO: use this as the default value for the global --binaries-path-by-id option! # panstd testing fails saying no run trackers were created when I tried to do this. SUPPORTED_PLATFORM_NORMALIZED_NAMES = { ('linux', 'x86_64'): ('linux', 'x86_64'), diff --git a/src/rust/engine/fs/src/lib.rs b/src/rust/engine/fs/src/lib.rs index afed6983104..fd00837630a 100644 --- a/src/rust/engine/fs/src/lib.rs +++ b/src/rust/engine/fs/src/lib.rs @@ -417,8 +417,8 @@ pub enum StrictGlobMatching { } impl StrictGlobMatching { - // TODO(cosmicexplorer): match this up with the allowed values for the GlobMatchErrorBehavior type - // in python somehow? + // TODO: match this up with the allowed values for the GlobMatchErrorBehavior type in python + // somehow? pub fn create(behavior: &str) -> Result { match behavior { "ignore" => Ok(StrictGlobMatching::Ignore), diff --git a/src/rust/engine/src/nodes.rs b/src/rust/engine/src/nodes.rs index 5ea728a845f..22877d411dc 100644 --- a/src/rust/engine/src/nodes.rs +++ b/src/rust/engine/src/nodes.rs @@ -1037,8 +1037,8 @@ impl Node for NodeKey { fn typstr(tc: &TypeConstraint) -> String { externs::key_to_str(&tc.0) } - // TODO(cosmicexplorer): these should all be converted to fmt::Debug implementations, and then - // this method can go away in favor of the auto-derived Debug for this type. + // TODO: these should all be converted to fmt::Debug implementations, and then this method can + // go away in favor of the auto-derived Debug for this type. match self { &NodeKey::DigestFile(ref s) => format!("DigestFile({:?})", s.0), &NodeKey::ExecuteProcess(ref s) => format!("ExecuteProcess({:?}", s.0), diff --git a/tests/python/pants_test/binaries/test_binary_tool.py b/tests/python/pants_test/binaries/test_binary_tool.py index 6e40b54ffa5..d43b4a639e1 100644 --- a/tests/python/pants_test/binaries/test_binary_tool.py +++ b/tests/python/pants_test/binaries/test_binary_tool.py @@ -72,8 +72,7 @@ def _select_for_version(self, version): return BinaryUtilFakeUname.Factory._create_for_cls(BinaryUtilFakeUname).select(binary_request) -# TODO(cosmicexplorer): these should have integration tests which use BinaryTool subclasses -# overriding archive_type +# TODO: these should have integration tests which use BinaryTool subclasses overriding archive_type. class BinaryToolBaseTest(BaseTest): def setUp(self): diff --git a/tests/python/pants_test/binaries/test_binary_util.py b/tests/python/pants_test/binaries/test_binary_util.py index 92ff4dc098f..d613eb16c88 100644 --- a/tests/python/pants_test/binaries/test_binary_util.py +++ b/tests/python/pants_test/binaries/test_binary_util.py @@ -32,7 +32,7 @@ def __str__(self): return 'ExternalUrlGenerator()' -# TODO(cosmicexplorer): test requests with an archiver! +# TODO: test requests with an archiver! class BinaryUtilTest(TestBase): """Tests binary_util's binaries_baseurls handling.""" diff --git a/tests/python/pants_test/engine/test_isolated_process.py b/tests/python/pants_test/engine/test_isolated_process.py index 2b734a9bee6..d53e10c35a3 100644 --- a/tests/python/pants_test/engine/test_isolated_process.py +++ b/tests/python/pants_test/engine/test_isolated_process.py @@ -217,8 +217,6 @@ def test_blows_up_on_invalid_args(self): with self.assertRaises(TypeCheckError): self._default_args_execute_process_request(argv=('1',), env=['foo', 'bar']) - # TODO(cosmicexplorer): we should probably check that the digest info in - # ExecuteProcessRequest is valid, beyond just checking if it's a string. with self.assertRaisesRegexp(TypeCheckError, "env"): ExecuteProcessRequest( argv=('1',), From 46fd3ac0d36749417f9b45b28a8b4484631719a2 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Sat, 15 Sep 2018 18:24:13 -0700 Subject: [PATCH 3/3] remove/clarify some TODOs --- src/python/pants/backend/native/subsystems/xcode_cli_tools.py | 2 +- src/python/pants/backend/native/targets/native_artifact.py | 3 ++- .../backend/native/tasks/native_external_library_fetch.py | 3 +-- src/python/pants/pantsd/watchman_launcher.py | 1 - src/rust/engine/fs/src/lib.rs | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/native/subsystems/xcode_cli_tools.py b/src/python/pants/backend/native/subsystems/xcode_cli_tools.py index 80bebb5d4dd..bdba39c75d5 100644 --- a/src/python/pants/backend/native/subsystems/xcode_cli_tools.py +++ b/src/python/pants/backend/native/subsystems/xcode_cli_tools.py @@ -128,7 +128,7 @@ def include_dirs(self): all_inc_dirs = base_inc_dirs for d in base_inc_dirs: - # TODO: what does this directory do? + # TODO: figure out what this directory does and why it's not already found by this compiler. secure_inc_dir = os.path.join(d, 'secure') if is_readable_dir(secure_inc_dir): all_inc_dirs.append(secure_inc_dir) diff --git a/src/python/pants/backend/native/targets/native_artifact.py b/src/python/pants/backend/native/targets/native_artifact.py index 396f5fdbb0f..dc8461d642c 100644 --- a/src/python/pants/backend/native/targets/native_artifact.py +++ b/src/python/pants/backend/native/targets/native_artifact.py @@ -28,6 +28,7 @@ def as_shared_lib(self, platform): }) def _compute_fingerprint(self): - # TODO: can we just use the __hash__ method here somehow? + # TODO: This fingerprint computation boilerplate is error-prone and could probably be + # streamlined, for simple payload fields. hasher = sha1(self.lib_name.encode('utf-8')) return hasher.hexdigest() if PY3 else hasher.hexdigest().decode('utf-8') diff --git a/src/python/pants/backend/native/tasks/native_external_library_fetch.py b/src/python/pants/backend/native/tasks/native_external_library_fetch.py index dc1d5c21d85..0eb75ce820c 100644 --- a/src/python/pants/backend/native/tasks/native_external_library_fetch.py +++ b/src/python/pants/backend/native/tasks/native_external_library_fetch.py @@ -33,8 +33,7 @@ class ConanRequirement(datatype(['pkg_spec'])): } def parse_conan_stdout_for_pkg_sha(self, stdout): - # TODO: properly regex this method. - # https://github.com/pantsbuild/pants/issues/6168 + # TODO(#6168): properly regex this method. pkg_line = stdout.split('Packages')[1] collected_matches = [line for line in pkg_line.split() if self.pkg_spec in line] pkg_sha = collected_matches[0].split(':')[1] diff --git a/src/python/pants/pantsd/watchman_launcher.py b/src/python/pants/pantsd/watchman_launcher.py index a8987276b3f..649711fefb4 100644 --- a/src/python/pants/pantsd/watchman_launcher.py +++ b/src/python/pants/pantsd/watchman_launcher.py @@ -27,7 +27,6 @@ def create(cls, bootstrap_options): baseurls=bootstrap_options.binaries_baseurls, binary_tool_fetcher=binary_tool_fetcher, path_by_id=bootstrap_options.binaries_path_by_id, - # TODO: do we need to test this? allow_external_binary_tool_downloads=bootstrap_options.allow_external_binary_tool_downloads) return WatchmanLauncher( diff --git a/src/rust/engine/fs/src/lib.rs b/src/rust/engine/fs/src/lib.rs index fd00837630a..b6a8bbd630e 100644 --- a/src/rust/engine/fs/src/lib.rs +++ b/src/rust/engine/fs/src/lib.rs @@ -418,7 +418,7 @@ pub enum StrictGlobMatching { impl StrictGlobMatching { // TODO: match this up with the allowed values for the GlobMatchErrorBehavior type in python - // somehow? + // somehow! pub fn create(behavior: &str) -> Result { match behavior { "ignore" => Ok(StrictGlobMatching::Ignore),