refactor: Separate domain metadata additions and removals #1421
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1421 +/- ##
==========================================
- Coverage 84.79% 84.78% -0.01%
==========================================
Files 118 118
Lines 30366 30376 +10
Branches 30366 30376 +10
==========================================
+ Hits 25748 25755 +7
- Misses 3387 3390 +3
Partials 1231 1231 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kernel/src/transaction/mod.rs
Outdated
| domain_metadatas_to_add: Vec<DomainMetadata>, | ||
| // Domain names to remove in this transaction. The configuration values are fetched during | ||
| // commit from the log to preserve the pre-image in tombstones. | ||
| domains_to_remove: Vec<String>, |
There was a problem hiding this comment.
add_domain_metadata, remove_domains ?
| .domain_metadata_additions | ||
| .clone() | ||
| .into_iter() |
There was a problem hiding this comment.
hm.. cloning everything since this method takes &self? is it only used in commit (which takes self)? if so should we just make this take self too?
There was a problem hiding this comment.
commit uses self later in the function, so we'd need to use &mut self + std::mem::take(), but it gets annoying because:
- commit would need to be mut self
- We'd leave the transaction in a partially-moved state mid-function
There was a problem hiding this comment.
Will leave as is, with room for optimization later
3 participants
What changes are proposed in this pull request?
This PR refactors the internal state management for domain metadata operations to separate additions and removals, cleaning up the internal representation.
How was this change tested?
Existing tests