Skip to content

Conversation

@hamishwillee
Copy link
Contributor

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.

@hamishwillee hamishwillee requested a review from MaEtUgR December 24, 2025 05:01

## 2. Field Documentation Standards

- **Inline Comments:** Every field and constant should have a comment on the same line, separated by exactly one space.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

every field no exceptions?

Copy link
Contributor Author

@hamishwillee hamishwillee Jan 2, 2026

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:

Suggested change
- **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.

Comment on lines +44 to +47
## 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.
Copy link
Contributor Author

@hamishwillee hamishwillee Jan 2, 2026

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

Suggested change
## 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants