-
Notifications
You must be signed in to change notification settings - Fork 14.9k
copilot: Review instructions for uorb msgs #26169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| ## 2. Field Documentation Standards | ||
|
|
||
| - **Inline Comments:** Every field and constant should have a comment on the same line, separated by exactly one space. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
every field no exceptions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have "should" so it isn't "must". But I guess "every" does imply that it is must, and we do have a few exceptions on constants.
How about:
| - **Inline Comments:** Every field and constant should have a comment on the same line, separated by exactly one space. | |
| - **Inline Comments:** Fields and constants should have a comment on the same line, separated by exactly one space. |
| ## 5. Deprecation & Semantic Changes | ||
|
|
||
| - **Field Deprecation:** Do not delete fields. Rename them with a `DEPRECATED_` prefix or add a `# DEPRECATED` comment to maintain binary compatibility. | ||
| - **Semantic Changes:** If a unit changes (e.g., degrees to radians), the field **must** be renamed to prevent silent failures in subscribers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dakejahl How about somthing like
| ## 5. Deprecation & Semantic Changes | |
| - **Field Deprecation:** Do not delete fields. Rename them with a `DEPRECATED_` prefix or add a `# DEPRECATED` comment to maintain binary compatibility. | |
| - **Semantic Changes:** If a unit changes (e.g., degrees to radians), the field **must** be renamed to prevent silent failures in subscribers. | |
| ## 5. Field Changes | |
| - If field name or type changes, or if a field is deleted or added, the message must contain a field named `MESSAGE_VERSION`. If the field did not exist previously it can have the value 1. If the field existed previously it should be incremented by 1. |
This uses gemini to write some copilot review instructions based on https://docs.px4.io/main/en/uorb/uorb_documentation
The idea is that copilot should do basic review only any PR that includes msg.
I have not checked this carefully - was hoping to iterate.