-
-
Notifications
You must be signed in to change notification settings - Fork 676
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3078 +/- ##
==========================================
- Coverage 65.35% 65.34% -0.01%
==========================================
Files 507 508 +1
Lines 57285 57388 +103
==========================================
+ Hits 37440 37502 +62
- Misses 15969 16003 +34
- Partials 3876 3883 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I'd really appreciate being able to use this if it were to be merged. Is there anything specific holding it up? |
df495ca
to
fb91d60
Compare
@kuhnchris can you take a look at the linter issues? |
Signed-off-by: Sijmen <me@sijman.nl> Signed-off-by: Sijmen Schoon <me@sijman.nl>
Signed-off-by: Sijmen <me@sijman.nl> Signed-off-by: Sijmen Schoon <me@sijman.nl>
Signed-off-by: Sijmen <me@sijman.nl> Signed-off-by: Sijmen Schoon <me@sijman.nl>
Signed-off-by: Sijmen <me@sijman.nl> Signed-off-by: Sijmen Schoon <me@sijman.nl>
Signed-off-by: Sijmen <me@sijman.nl> Signed-off-by: Sijmen Schoon <me@sijman.nl>
Signed-off-by: Sijmen Schoon <me@sijman.nl>
e210ffb
to
579eb2f
Compare
Done, now the linter is... broken? |
No idea why this fixes the issue, it just did...
|
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.
A few minor things, but aside from that looks good.
Also, thanks for adding tests! :)
It's not my code, it's all thanks to @vijfhoek s work, I only did the ff/rebase work, hence I only have limited knowledge of the change, plus, my go know-how is still very limited, but I try my best to fix whatever comes up. :-) |
Co-authored-by: Till <2353100+S7evinK@users.noreply.github.com>
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.
A bit late for a review, but the notification about the merge of main
made me notice you can just do
if len(cfg.Derived.ApplicationServices) > 0 {
loginFlows = append(loginFlows, flow{Type: authtypes.LoginTypeApplicationService})
}
and skip the zero length check since you're already declaring the loginFlows
slice with authtypes.LoginTypePassword
before the conditional statements
@devonh I was basically happy with this, but maybe you can also have a look? |
Let's finally land this.. Issue for the flake in Complement: #3273 |
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.
Edit: Comment was not intentional. :D
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.
Changes look thought out and have decent test coverage.
Thank you very much @kuhnchris and @vijfhoek for this! |
Nice, thanks for the merge, in the meantime I'll try to get the mautrix bridges working, which indirectly also is affected by appservices, but I'll create a separate PR for that - again thanks for pushing through with it. :-) 👍 |
Does that mean that End-to-bridge encryption is now working with mautrix bridges and dendrite (master)? I think, I won't need this, because I'm planning to run both the mautrix bridges as well as the dendrite homeserver on the same machine, but it would be nice for those who don't. |
Rebase of #2936 as @vijfhoek wrote he got no time to work on this, and I kind of needed it for my experiments.
I checked the tests, and it is working with my example code (i.e. impersonating, registering, creating channel, invite people, write messages).
I'm not a huge
go
pro, and still learning, but I tried to fix and/or integrate the changes as best as possible with the currentmain
branch changes.If there is anything left, let me know and I'll try to figure it out.
Signed-off-by:
Kuhn Christopher <kuhnchris+git@kuhnchris.eu>