Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

Remove PerformError #3066

Merged
merged 15 commits into from
Apr 28, 2023
Merged

Remove PerformError #3066

merged 15 commits into from
Apr 28, 2023

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Apr 25, 2023

This removes PerformError, which was needed when we still had polylith.

This removes quite a bunch of

if err != nil {
	return err
}
if err := res.Error; err != nil {
	return err.JSONResponse()
}

Hopefully can be read commit by commit.

@S7evinK S7evinK added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Apr 25, 2023
@S7evinK S7evinK requested a review from a team as a code owner April 25, 2023 08:35
@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Patch coverage: 40.31% and project coverage change: +0.19 🎉

Comparison is base (d23d036) 66.57% compared to head (9c4a605) 66.77%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3066      +/-   ##
==========================================
+ Coverage   66.57%   66.77%   +0.19%     
==========================================
  Files         496      497       +1     
  Lines       53525    53200     -325     
==========================================
- Hits        35634    35522     -112     
+ Misses      14275    14101     -174     
+ Partials     3616     3577      -39     
Flag Coverage Δ
unittests 49.50% <40.31%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
clientapi/routing/directory.go 60.08% <0.00%> (+0.74%) ⬆️
clientapi/routing/peekroom.go 31.34% <0.00%> (-16.81%) ⬇️
clientapi/routing/upgrade_room.go 51.51% <0.00%> (-26.54%) ⬇️
federationapi/routing/invite.go 41.17% <0.00%> (-5.23%) ⬇️
roomserver/api/perform.go 100.00% <ø> (+11.36%) ⬆️
roomserver/internal/perform/perform_peek.go 42.20% <0.00%> (+1.35%) ⬆️
roomserver/internal/perform/perform_unpeek.go 0.00% <0.00%> (ø)
userapi/internal/user_api.go 62.87% <0.00%> (-0.26%) ⬇️
roomserver/internal/perform/perform_invite.go 74.09% <16.66%> (+1.40%) ⬆️
clientapi/routing/createroom.go 60.70% <37.50%> (-0.54%) ⬇️
... and 10 more

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearer API overall, LGTM!

Code: rsAPI.PerformErrorNoOperation,
Msg: fmt.Sprintf("InputRoomEvents failed: %s", err),
}
return "", "", rsAPI.ErrNotAllowed{Err: err}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably isn't a "not allowed" error? It used to be err no op.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PerformErrorNoOperation also returned a http.StatusForbidden in the end, so this should end up being the same error code for clients.

@S7evinK S7evinK merged commit 6b47cf0 into main Apr 28, 2023
@S7evinK S7evinK deleted the s7evink/performerr branch April 28, 2023 15:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants