-
Notifications
You must be signed in to change notification settings - Fork 314
refactor(FQDN): further more refator on idl/dsn.layer2.thrift v2 #2217
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
Conversation
42503a1
to
02eef41
Compare
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.
@Samunroyu pls check the failed tests, thanks!
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.
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
src/meta/server_state_restore.cpp:258
- The implicit boolean check on 'primary' may be ambiguous if host_port does not clearly define a boolean conversion. Consider explicitly checking whether 'primary' is set or valid (e.g., comparing against a default host_port instance).
if (primary || !secondaries.empty()) {
src/meta/meta_bulk_load_service.cpp:479
- [nitpick] The temporary variable 'primary' used here could be confused with other primary values in the context; consider renaming it to a more descriptive name such as 'extracted_primary' for clarity.
GET_HOST_PORT(request, primary, primary_hp);
src/meta/duplication/meta_duplication_service.cpp:771
- Relying on an implicit boolean check on 'primary' might lead to ambiguity; explicitly verify if 'primary' is valid or set to its default value to ensure correct behavior.
if (!primary) {
This is a further refactor on idl/dsn.layer2.thrift based on #2049
the main effects are on partition_configuration structure.