-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add cancellable field in the mint response #54
base: dev
Are you sure you want to change the base?
Conversation
4b27f0d
to
0ff3c68
Compare
📝 WalkthroughWalkthroughThe changes involve the addition of a new boolean field named Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MintService
participant MintResponse
Client->>MintService: Request booking
MintService-->>Client: Return MintResponse
MintResponse->>Client: {cancellable: true/false}
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 (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
proto/cmp/services/book/v3/mint.proto (2)
98-101
: Fix typo and enhance field documentationThe documentation has a typo and could be more comprehensive about the cancellation status determination.
Apply this diff to improve the documentation:
// This field indicates whether the booking can be cancelled. If this field - // is not used, the default value is false, whick means the booking cannot be + // is not used, the default value is false, which means the booking cannot be // cancelled. + // This status is determined by the supplier at booking time and remains immutable + // for the lifetime of the booking. bool cancellable = 12;
97-101
: Consider documenting on-chain implicationsSince this is part of an on-chain booking system, it would be helpful to document:
- How this flag affects smart contract behavior
- Whether additional on-chain validation is required
- Future considerations if cancellation requires more metadata (e.g., cancellation fees, deadlines)
Consider adding these details to the protocol documentation or creating a separate architectural decision record (ADR) to capture these design decisions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
proto/cmp/services/book/v3/mint.proto
(1 hunks)
🔇 Additional comments (1)
proto/cmp/services/book/v3/mint.proto (1)
97-101
: LGTM! Change is backward compatible and follows best practices
The addition of the cancellable
field is well-designed:
- Optional field with default value ensures backward compatibility
- Field number 12 maintains proper message evolution
- Placement in MintResponse is appropriate for this booking property
Let's verify no breaking changes were introduced:
✅ Verification successful
LGTM! Change is backward compatible and follows best practices
The verification confirms:
- No duplicate field numbers exist in MintResponse
- All fields from 1 to 11 are present and maintained
- Field number 12 for
cancellable
is correctly positioned without conflicts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify backward compatibility by checking for any reuse of field numbers
# or removal of existing fields in the MintResponse message
# Check for any duplicate field numbers in MintResponse
rg -A 1 "= \d+" proto/cmp/services/book/v3/mint.proto | grep "=" | sort -n -k3 | uniq -d -f2
# Verify all previous fields (1-11) are still present in MintResponse
for i in {1..11}; do
rg "= $i;" proto/cmp/services/book/v3/mint.proto || echo "Missing field number $i"
done
Length of output: 1786
@havan |
We added a field in the Mint response to indicate whether a booking can be cancelled. This is backward compatible amendment at the end of the file.
Summary by CodeRabbit