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

Ratelimit config code has a RateLimitConfig class and a RatelimitConfig one #4983

Closed
babolivier opened this issue Apr 1, 2019 · 2 comments · Fixed by #13442
Closed

Ratelimit config code has a RateLimitConfig class and a RatelimitConfig one #4983

babolivier opened this issue Apr 1, 2019 · 2 comments · Fixed by #13442
Labels
good first issue Good for newcomers P3 (OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patches S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution

Comments

@babolivier
Copy link
Contributor

This makes the config code harder to read/understand due to both classes being super easy to mistake for the other. One (probably RateLimitConfig) should be renamed into something else.
@neilisfragile neilisfragile added z-p3 (Deprecated Label) improvement labels Apr 1, 2019
@richvdh
Copy link
Member

richvdh commented Jun 15, 2022

still true, btw.

@richvdh richvdh added good first issue Good for newcomers P3 (OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patches S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution and removed z-enhancement z-p3 (Deprecated Label) labels Jun 15, 2022
@richvdh
Copy link
Member

richvdh commented Jun 15, 2022

I suggest renaming RateLimitConfig to RatelimitSettings

@dklimpel dklimpel mentioned this issue Aug 3, 2022
Merged
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
good first issue Good for newcomers P3 (OBSOLETE: use S- labels.) Approved backlog: not yet scheduled, will accept patches S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rename RateLimitConfig to RatelimitSettings dklimpel/synapse_archive
4 participants
@neilisfragile @richvdh @babolivier @erikjohnston