-
Notifications
You must be signed in to change notification settings - Fork 252
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
ffi: add previous
power levels to OtherState::RoomPowerLevels
#3199
ffi: add previous
power levels to OtherState::RoomPowerLevels
#3199
Conversation
8083402
to
40f5fa3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3199 +/- ##
=======================================
Coverage 83.83% 83.83%
=======================================
Files 232 232
Lines 23985 23985
=======================================
Hits 20109 20109
Misses 3876 3876 ☔ View full report in Codecov by Sentry. |
This is needed to be able to diff between increases and decreases of power levels.
40f5fa3
to
ead2e14
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.
lgtm!
FullContent::Original { content, prev_content } => { | ||
power_level_user_changes(content, prev_content) | ||
previous = prev_content.as_ref().map(|prev_content| { |
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.
style nit: can you have each branch return a Self::RoomPowerLevels
immediately, instead of having two temporary variables? it would read slightly more straightforward.
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.
Sure, I just did it this way to not repeat too much code.
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.
Done in ff911e4
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.
Oops, sorry I meant the opposite: have each arm return a Self::RoomPowerLevels
. I'll make the change.
This is needed to be able to diff between increases and decreases of power levels ("user Alice was promoted Admin", etc.).
Signed-off-by: