Copilot commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3584086193
##########
AGENTS.md:
##########
@@ -313,7 +316,11 @@ SMDebug(dbg_ctl, "Processing request for URL: %s", url);
- CamelCase for classes: `HttpSM`, `NetVConnection`
- snake_case for variables and functions: `server_entry`, `handle_api_return()`
- UPPER_CASE for macros and constants: `HTTP_SM_SET_DEFAULT_HANDLER`
-- Private member variables have the `m_` prefix.
+- Private member variables use both `_` and `m_` prefixes in the source code;
+ be consistent with the surrounding code. For new files, prefer the `_` prefix
+ because that reflects the community consensus.
+- Boolean variables and functions returning boolean values are named as
+ predicates: use `was_modified` rather than `modified`.
Review Comment:
This guidance says private members use an “`_` prefix”, but the codebase
examples are overwhelmingly *trailing* underscores (e.g. `state_`, `name_`)
plus `m_`-prefixed members (e.g. `m_ip_pool`). Also, recommending
leading-underscore names in new code is risky because leading underscores can
be reserved in some contexts. Consider rewording to “trailing `_` suffix” +
`m_` prefix and remove the “community consensus” phrasing.
##########
.github/copilot-instructions.md:
##########
@@ -74,7 +74,11 @@ void function() {
- Classes: `CamelCase` → `HttpSM`, `NetVConnection`, `CacheProcessor`
- Functions/variables: `snake_case` → `handle_request()`, `server_port`,
`cache_key`
- Constants/macros: `UPPER_CASE` → `HTTP_STATUS_OK`, `MAX_BUFFER_SIZE`
-- Member variables: `snake_case` with no prefix → `connection_count`,
`buffer_size`
+- Private member variables use both `_` and `m_` prefixes in the source code;
+ be consistent with the surrounding code. For new files, prefer the `_` prefix
+ because that reflects the community consensus.
+- Boolean variables and functions returning boolean values are named as
+ predicates: use `was_modified` rather than `modified`.
Review Comment:
These naming bullets describe an “`_` prefix” for private members and use
`was_modified` as the boolean predicate example. In the codebase, private
members commonly use a trailing underscore suffix (e.g. `state_`, `name_`) and
`m_`-prefixed members are also common; `was_modified` doesn’t appear to be
used. Rewording to “trailing `_` suffix” and using a more conventional
predicate example (e.g. `is_modified`) would make this guidance match existing
patterns and avoid implying leading-underscore identifiers.
##########
.github/copilot-instructions.md:
##########
@@ -153,6 +157,7 @@ for (auto &conn : connections) {
- Python 3.11+ with type hints
- 4-space indentation (never tabs)
- Type annotations on all function signatures
+- Use f'...' strings rather than .format() strings for new Python code.
Review Comment:
This line says “Use `f'...'` strings”, which can be read as requiring single
quotes specifically (and excludes multi-line f-strings). It’s clearer and more
accurate to say “Use f-strings” rather than calling out one literal form.
--
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]
