Conversation
update of newdomainevent schema Signed-off-by: Ulrika Vincent <91091781+ulvincent@users.noreply.github.com>
update of new domain event schema Signed-off-by: Ulrika Vincent <91091781+ulvincent@users.noreply.github.com>
WalkthroughThe pull request introduces a comprehensive restructuring of the "New Domain" event notification data fields in the NewDomainEvent.md document. The changes involve renaming existing fields, modifying their requirements, and adding a new field called "ProtocolVersion". The modifications impact the field signatures, their order, and mandatory status, with a focus on prefixing some fields with "Q" and adjusting the overall event structure. Additionally, the TAPIRDataFlow.md document has been simplified by removing a detailed mermaid diagram and replacing it with a link to an external document for data flow information. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
NewDomainEvent.md(1 hunks)
🔇 Additional comments (1)
NewDomainEvent.md (1)
16-17:⚠️ Potential issueAdd missing field descriptions.
The following fields lack descriptions:
- Type: Purpose and usage need to be documented
- ProtocolVersion: Version information and format need to be specified
Please provide descriptions for these fields. Additionally, since Type appears to be repurposed from its original meaning (based on the AI summary), its new purpose should be clearly documented to avoid confusion.
| | 1 | Flags | Integer | yes | Response Header flags | | ||
| | 2 | Timestamp | Timestamp | yes | Event time of transmission | | ||
| | 5 | QClass | Int16 | no | Query Class | | ||
| | 3 | QName | Bytestring | yes | The fully qualified domain name | | ||
| | 4 | QType | Int16 | yes | Query type | | ||
| | 8 | RdLength | Int16 | yes | Length of RDATA | | ||
| | 8 | Type | Int16 | yes | | | ||
| | 11 | ProtocolVersion | Int16 | yes | | |
There was a problem hiding this comment.
Fix field numbering and ordering inconsistencies.
The field ordering and numbering have several issues:
- Field numbers are not sequential (jumps from 5 to 8 to 11)
- Duplicate field number 8 used for both RdLength and Type
- Related fields (QName, QType, QClass) are scattered instead of grouped together
Apply this diff to fix the ordering and numbering:
| # | Name | Type | Required | Description|
|:---| :------------- | :------------- |:-------- | :------------- |
-| 1 | Flags | Integer | yes | Response Header flags |
-| 2 | Timestamp | Timestamp | yes | Event time of transmission |
-| 5 | QClass | Int16 | no | Query Class |
-| 3 | QName | Bytestring | yes | The fully qualified domain name |
-| 4 | QType | Int16 | yes | Query type |
-| 8 | RdLength | Int16 | yes | Length of RDATA |
-| 8 | Type | Int16 | yes | |
-| 11 | ProtocolVersion | Int16 | yes | |
+| 1 | Flags | Integer | yes | Response Header flags |
+| 2 | Timestamp | Timestamp | yes | Event time of transmission |
+| 3 | QName | Bytestring | yes | The fully qualified domain name |
+| 4 | QType | Int16 | yes | Query type |
+| 5 | QClass | Int16 | no | Query Class |
+| 6 | RdLength | Int16 | yes | Length of RDATA |
+| 7 | Type | Int16 | yes | |
+| 8 | ProtocolVersion | Int16 | yes | |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | 1 | Flags | Integer | yes | Response Header flags | | |
| | 2 | Timestamp | Timestamp | yes | Event time of transmission | | |
| | 5 | QClass | Int16 | no | Query Class | | |
| | 3 | QName | Bytestring | yes | The fully qualified domain name | | |
| | 4 | QType | Int16 | yes | Query type | | |
| | 8 | RdLength | Int16 | yes | Length of RDATA | | |
| | 8 | Type | Int16 | yes | | | |
| | 11 | ProtocolVersion | Int16 | yes | | | |
| | 1 | Flags | Integer | yes | Response Header flags | | |
| | 2 | Timestamp | Timestamp | yes | Event time of transmission | | |
| | 3 | QName | Bytestring | yes | The fully qualified domain name | | |
| | 4 | QType | Int16 | yes | Query type | | |
| | 5 | QClass | Int16 | no | Query Class | | |
| | 6 | RdLength | Int16 | yes | Length of RDATA | | |
| | 7 | Type | Int16 | yes | | | |
| | 8 | ProtocolVersion | Int16 | yes | | |
refered to diagram on web site Signed-off-by: Ulrika Vincent <91091781+ulvincent@users.noreply.github.com>
|
also updated dataflow.md to refer to dataflow on website |
3 participants
minor update in histogram and new domain event schema description
Summary by CodeRabbit
ProtocolVersionfield to domain event notificationsEvent→Flags,Sendtime→TimestampNametoQName,TypetoQType, andClasstoQClassTimestampa required field