You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2026/06/25 16:34:17 UTC
[PR] AI agent instruction updates [trafficserver]
bneradt opened a new pull request, #13335:
URL: https://github.com/apache/trafficserver/pull/13335
A few tweaks for guiding AI agents.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3584234770
##########
.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:
> private members commonly use a trailing underscore suffix (e.g. state_, name_)
So you say...I tend to see more `_` and `m_` prefixed variables. But maybe that's just the code I've worked with more.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3584072899
##########
.github/copilot-instructions.md:
##########
@@ -75,6 +75,8 @@ void function() {
- 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: `snake_case` with `m_` prefix → `m_was_modified`
Review Comment:
Updated. Thanks for the feedback.
Since both _ and m_ are used in the source, so the guidance now says to follow the surrounding code for consistency. For new files, it prefers _ to reflect the community consensus.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3476337234
##########
.github/copilot-instructions.md:
##########
@@ -75,6 +75,8 @@ void function() {
- 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: `snake_case` with `m_` prefix → `m_was_modified`
Review Comment:
It is sadly common, but generally not advised because of possible conflicts with other internal variables. `m_` is the typical pattern, and is also not uncommon in our codebase. We're really better off with `m_` prefixes for new variables.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3587147136
##########
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:
Thanks for the reference.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3478160868
##########
.github/copilot-instructions.md:
##########
@@ -75,6 +75,8 @@ void function() {
- 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: `snake_case` with `m_` prefix → `m_was_modified`
Review Comment:
FWIW, I was following `_` prefix rule because it's written in here to do so.
https://cwiki.apache.org/confluence/display/TS/Coding+Style#CodingStyle-MemberVariables
Probably, it's time to discuss this bike-shedding :)
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3584235996
##########
.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:
Meh. People will get the point I think.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3476380632
##########
.github/copilot-instructions.md:
##########
@@ -75,6 +75,8 @@ void function() {
- 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: `snake_case` with `m_` prefix → `m_was_modified`
Review Comment:
For instance:
https://stackoverflow.com/a/20496955/629530
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3584083305
##########
AGENTS.md:
##########
@@ -313,7 +316,10 @@ 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.
+- Booleans variables and functions returning booleans are named as predicates: not `modified` but `was_modified`
Review Comment:
fixed
##########
.github/copilot-instructions.md:
##########
@@ -74,7 +74,10 @@ 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.
+- Booleans variables are predicates: not `modified` but `was_modified`
Review Comment:
fixed
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3584231086
##########
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:
> Also, recommending leading-underscore names in new code is risky because leading underscores can be reserved in some contexts.
Yeah, you and I lost that argument. The community prefers `_` prefixes for private variables.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3476174461
##########
.github/copilot-instructions.md:
##########
@@ -75,6 +75,8 @@ void function() {
- 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: `snake_case` with `m_` prefix → `m_was_modified`
Review Comment:
I did a quick check and it looks like the leading underscore `_foo` style is the most common in the existing code. About 71% of private member variables. Could we keep using that?
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3586919996
##########
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:
It seems identifiers with leading `_` prefixes followed by lowercase letters are [only reserved if they have external linkage](https://en.cppreference.com/c/language/identifier).
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AI agent instruction updates [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13335:
URL: https://github.com/apache/trafficserver/pull/13335#discussion_r3583182282
##########
AGENTS.md:
##########
@@ -313,7 +316,10 @@ 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.
+- Booleans variables and functions returning booleans are named as predicates: not `modified` but `was_modified`
Review Comment:
Grammar/wording in this bullet is off ("Booleans variables"), and the line is very long compared to surrounding wrapped bullets. Consider correcting to "Boolean variables" (and "boolean values") and wrapping the example onto a continuation line for readability.
##########
.github/copilot-instructions.md:
##########
@@ -74,7 +74,10 @@ 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.
+- Booleans variables are predicates: not `modified` but `was_modified`
Review Comment:
This bullet has a grammatical error ("Booleans variables"). It should be "Boolean variables".
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org