Copilot commented on code in PR #12957:
URL: https://github.com/apache/trafficserver/pull/12957#discussion_r2922570448
##########
tools/hrw4u/Makefile:
##########
@@ -167,6 +167,14 @@ $(PKG_DIR_LSP)/%: src/%
test:
uv run pytest --tb=short tests
+coverage:
+ uv run pytest --cov --cov-report=term-missing --cov-report=html tests
+ @echo ""
+ @echo "HTML report: open htmlcov/index.html"
+
+coverage-open: coverage
+ open htmlcov/index.html
Review Comment:
`coverage-open` uses the `open` command, which is macOS-specific and will
fail on Linux/BSD environments. Consider using a more portable approach (e.g.,
`python -m webbrowser htmlcov/index.html`, or conditional `xdg-open`/`open`) so
the target works across common dev/CI platforms.
```suggestion
@echo "HTML report: use 'make coverage-open' or open htmlcov/index.html
in a browser"
coverage-open: coverage
uv run python -m webbrowser htmlcov/index.html
```
##########
tools/hrw4u/src/common.py:
##########
@@ -112,12 +112,7 @@ def fatal(message: str) -> NoReturn:
def create_base_parser(description: str) -> tuple[argparse.ArgumentParser,
argparse._MutuallyExclusiveGroup]:
"""Create base argument parser with common options."""
parser = argparse.ArgumentParser(description=description,
formatter_class=argparse.RawDescriptionHelpFormatter)
- parser.add_argument(
- "input_file",
- help="The input file to parse (default: stdin)",
- nargs="?",
- type=argparse.FileType("r", encoding="utf-8"),
- default=sys.stdin)
+ parser.add_argument("input_file", help="The input file to parse (default:
stdin)", nargs="?", default=None)
Review Comment:
`create_base_parser()` advertises "default: stdin" but the new `input_file`
argument defaults to `None` and no longer uses `argparse.FileType`. That makes
`args.input_file` inconsistent with `process_input()` (which expects a
`TextIO`) and breaks any callers that previously relied on getting an open file
handle / stdin fallback. Consider restoring `type=argparse.FileType(...),
default=sys.stdin` or updating the argument name/help/type annotations and any
downstream usage to reflect that this is now a path/None rather than a file
object.
```suggestion
parser.add_argument(
"input_file",
help="The input file to parse (default: stdin)",
nargs="?",
type=argparse.FileType("r"),
default=sys.stdin,
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
