style: Remove implement and related macros #1880

Merged
nex merged 44 commits from nex/style/remove-implement into main 2026-06-27 17:54:16 +00:00
Owner

The implement macro has long been a hindrance. It was supposed to replace the impl Struct pattern, 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_command admin crate macro also depended on this function, so the admin crate was refactored to swap out admin_command for impl crate::Context (which is what the admin_command macro 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-style commits. Everything else is just shifting a #[implement(...)] or #[admin_command] to impl ....

Pull request checklist:

  • This pull request targets the main branch, and the branch is named something other than
    main.
  • I have written an appropriate pull request title and my description is clear.
  • I understand I am responsible for the contents of this pull request.
  • I have followed the contributing guidelines:
The `implement` macro has long been a hindrance. It was supposed to replace the `impl Struct` pattern, 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_command` admin crate macro also depended on this function, so the admin crate was refactored to swap out `admin_command` for `impl crate::Context` (which is what the `admin_command` macro 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-`style` commits. Everything else is just shifting a `#[implement(...)]` or `#[admin_command]` to `impl ...`. **Pull request checklist:** <!-- You need to complete these before your PR can be considered. If you aren't sure about some, feel free to ask for clarification in #dev:continuwuity.org. --> - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [x] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. <!-- Notes on these requirements: - While not required, we encourage you to sign your commits with GPG or SSH to attest the authenticity of your changes. - While we allow LLM-assisted contributions, we do not appreciate contributions that are low quality, which is typical of machine-generated contributions that have not had a lot of love and care from a human. Please do not open a PR if all you have done is asked ChatGPT to tidy up the codebase with a +-100,000 diff. - In the case of code style violations, reviewers may leave review comments/change requests indicating what the ideal change would look like. For example, a reviewer may suggest you lower a log level, or use `match` instead of `if/else` etc. - In the case of code style violations, pre-commit check failures, minor things like typos/spelling errors, and in some cases commit format violations, reviewers may modify your branch directly, typically by making changes and adding a commit. Particularly in the latter case, a reviewer may rebase your commits to squash "spammy" ones (like "fix", "fix", "actually fix"), and reword commit messages that don't satisfy the format. - Pull requests MUST pass the `Checks` CI workflows to be capable of being merged. This can only be bypassed in exceptional circumstances. If your CI flakes, let us know in matrix:r/dev:continuwuity.org. - Pull requests have to be based on the latest `main` commit before being merged. If the main branch changes while you're making your changes, you should make sure you rebase on main before opening a PR. Your branch will be rebased on main before it is merged if it has fallen behind. - We typically only do fast-forward merges, so your entire commit log will be included. Once in main, it's difficult to get out cleanly, so put on your best dress, smile for the cameras! --> [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages
nex added 38 commits 2026-06-26 06:28:46 +00:00
good riddance to this thing
Also changes the default power levels so people stop shooting their feet off
style: Refactor admin crate, drop admin_command macro
All checks were successful
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 5s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m1s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m16s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m39s
f7ef9bc0dd
`admin_command` depended on `implement`, which is now gone.
nex changed title from nex/style/remove-implement to style: Remove implement and related macros 2026-06-26 06:29:02 +00:00
Also resolvers several lints, including needless async on create_user
I won't even comment on how stupid that naming was.
refactor: Add better documentation to sender service, add additional logging traces
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 9s
Checks / Prek / Check changed files (pull_request) Successful in 8s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m0s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m19s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 4m20s
e3b1ca1f7a
This is a surprise tool that will help us later!
Author
Owner

There's a bugfix here, previously the wrong user ID was checked a second time

There's a bugfix here, previously the wrong user ID was checked a second time
Author
Owner

sort of logic change, this parallelises the user and AS lookup

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 level
if *pdu.kind() != TimelineEventType::RoomCreate {
tokio::join!(
Author
Owner

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

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_appservices
Author
Owner

This to is a logic change, appservices are now pushed to in parallel, although I doubt there's a huge gain considering send_pdu_appservice itself isn't async

This to is a logic change, appservices are now pushed to in parallel, although I doubt there's a huge gain considering `send_pdu_appservice` itself 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, not
Author
Owner

This TODO is new (but out of scope for this PR)

This 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!(
Author
Owner

The two futures are joined here, they were not previously

The two futures are joined here, they were not previously
@ -439,3 +456,1 @@
val.deserialize_as_unchecked().map_err(Into::into)
})
}
match states {
Author
Owner

I hate this match function but whatever im not adding more lines to the diff

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| {
Author
Owner

I originally wrote a TODO here before deciding to just DO it. This change specifically prioritises any text/plain extensible block in the room topic, falling back to the original top-level topic string.

I originally wrote a TODO here before deciding to just DO it. This change specifically prioritises any `text/plain` extensible block in the room topic, falling back to the original top-level `topic` string.
@ -225,0 +156,4 @@
Ok((count, pdus))
}
// TODO: The next 4 functions can be, theoretically, squashed into one.
Author
Owner

new TODO, but out of scope

new TODO, but out of scope
@ -1,20 +1,22 @@
use std::collections::BTreeMap;
use conduwuit::{Result, info, pdu::PartialPdu};
Author
Owner

This entire commit is a feature unrelated to the refactoring, but I figured it was cheap enough to include anyway.

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 level
Author
Owner

This is a new feature to complement the previous functionality upgrade, user power levels are dropped when they're removed from the admin room

This 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 {
Author
Owner

The only change in this commit that isn't strictly removing implement is renaming check to assert

The only change in this commit that isn't strictly removing `implement` is renaming `check` to `assert`
fix: Load bearing mut refs
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m2s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 10m26s
e751a96f55
Jade approved these changes 2026-06-26 18:32:18 +00:00
nex force-pushed nex/style/remove-implement from e751a96f55
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m2s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 10m26s
to 2d65d47498
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m19s
Checks / Prek / Check changed files (pull_request) Successful in 8s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m3s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
2026-06-27 17:37:40 +00:00
Compare
style: Reformat
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Checks / Prek / Check changed files (pull_request) Successful in 8s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m18s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m15s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m0s
Checks / Prek / Check changed files (push) Successful in 7s
Documentation / Build and Deploy Documentation (push) Successful in 1m2s
Checks / Prek / Pre-commit & Formatting (push) Successful in 1m22s
Checks / Prek / Clippy and Cargo Tests (push) Successful in 9m3s
Release Docker Image / Build linux-amd64 (release) (push) Successful in 11m34s
Release Docker Image / Build linux-arm64 (max-perf) (push) Has been skipped
Release Docker Image / Build linux-arm64 (release) (push) Successful in 10m45s
Release Docker Image / Create Multi-arch Release Manifest (push) Successful in 18s
Release Docker Image / Build linux-amd64 (max-perf) (push) Successful in 27m40s
Release Docker Image / Create Max-Perf Manifest (push) Has been skipped
Release Docker Image / Mirror Images (push) Has been skipped
Release Docker Image / Release Binaries (push) Has been skipped
458e0b53ac
nex merged commit 458e0b53ac into main 2026-06-27 17:54:16 +00:00
nex deleted branch nex/style/remove-implement 2026-06-27 17:54:16 +00:00
ginger added this to the 26.6.0 milestone 2026-07-04 15:57:01 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
continuwuation/continuwuity!1880
No description provided.