style: Remove implement and related macros #1880
No reviewers
Labels
No labels
Abandoned
Blocked
Bug
Changelog
Added
Changelog
Missing
Changelog
None
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/E2EE
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Matrix/T&S
Merge
Merge/Manual
Merge/Squash
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
continuwuation/continuwuity!1880
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "nex/style/remove-implement"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The
implementmacro has long been a hindrance. It was supposed to replace theimpl Structpattern, but it was used inconsistently, and it seemingly caused issues with intellisense and further complicated the code. This PR removes that macro entirely.the
admin_commandadmin crate macro also depended on this function, so the admin crate was refactored to swap outadmin_commandforimpl crate::Context(which is what theadmin_commandmacro was a shortcut for).The PR diff is absolutely huge and Forgejo probably won't be able to render it. I recommend going through each commit one by one (which isn't much better, but it's still less than 30k lines). The only functional changes are in non-
stylecommits. Everything else is just shifting a#[implement(...)]or#[admin_command]toimpl ....Pull request checklist:
mainbranch, and the branch is named something other thanmain.myself, if applicable. This includes ensuring code compiles.
implinstead ofimplementin moderation service d13acc948eimplementfor AccessCheck, renamechecktoassert46852280adimplementfor configManager894a43030bimplementfor database pool 7e1aa49069implementin database map keys d941bf1220implementdatabase 824919b890implement40a0698b5bimplementimports 6e9b42341aimplemententirely 4a353d2d54admin_commandmacronex/style/remove-implementto style: Removeimplementand related macrosThere's a bugfix here, previously the wrong user ID was checked a second time
sort of logic change, this parallelises the user and AS lookup
@ -197,0 +209,4 @@// See if the event matches any known pushers via power levelif *pdu.kind() != TimelineEventType::RoomCreate {tokio::join!(This is a logic change: notifying pushers, handling PDU effects (redaction etc), and aggregating relations are all handled in parallel and are also made infallible
@ -435,0 +495,4 @@.values().map(ToOwned::to_owned) // TODO: is this to_owned expensive?.collect::<Vec<_>>();interested_appservicesThis to is a logic change, appservices are now pushed to in parallel, although I doubt there's a huge gain considering
send_pdu_appserviceitself isn't async@ -346,0 +272,4 @@/// immutable. Since the PDU is immutable, the `event_id` field is/// populated.////// TODO: The `event_id` field should be a separate return option, notThis TODO is new (but out of scope for this PR)
@ -107,0 +119,4 @@return false;};let (as_user_joined, any_as_namespace_user_joined) = tokio::join!(The two futures are joined here, they were not previously
@ -439,3 +456,1 @@val.deserialize_as_unchecked().map_err(Into::into)})}match states {I hate this match function but whatever im not adding more lines to the diff
@ -125,3 +128,3 @@self.room_state_get_content(room_id, &StateEventType::RoomTopic, "").await.map(|c: RoomTopicEventContent| c.topic).map(|c: RoomTopicEventContent| {I originally wrote a TODO here before deciding to just DO it. This change specifically prioritises any
text/plainextensible block in the room topic, falling back to the original top-leveltopicstring.@ -225,0 +156,4 @@Ok((count, pdus))}// TODO: The next 4 functions can be, theoretically, squashed into one.new TODO, but out of scope
@ -1,20 +1,22 @@use std::collections::BTreeMap;use conduwuit::{Result, info, pdu::PartialPdu};This entire commit is a feature unrelated to the refactoring, but I figured it was cheap enough to include anyway.
@ -217,0 +189,4 @@.await.expect("admin room should have power levels");if room_power_levels.users.remove(user_id).is_some() {// drop the target's power levelThis is a new feature to complement the previous functionality upgrade, user power levels are dropped when they're removed from the admin room
@ -21,0 +14,4 @@/// Asserts that the server has access to the room and event (if any)./// If the server is permitted, `Ok(())` is returned. Otherwise, a Forbidden/// error is returned.pub(super) async fn assert(&self) -> Result {The only change in this commit that isn't strictly removing
implementis renamingchecktoasserte751a96f552d65d47498