Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CHIA-2065 Patch outbound_rate_limiter in test_spam_message_too_large #19038

Draft
wants to merge 1 commit into
base: test-cleanup
Choose a base branch
from
Draft

CHIA-2065 Patch outbound_rate_limiter in test_spam_message_too_large #19038

wants to merge 1 commit into from

Conversation

AmineKhaldi
Copy link
Contributor

@AmineKhaldi AmineKhaldi commented Dec 12, 2024 •
edited
Loading

Addresses #19036 (comment).

@AmineKhaldi AmineKhaldi added Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Cleanup Code cleanup labels Dec 12, 2024
@AmineKhaldi AmineKhaldi self-assigned this Dec 12, 2024
@AmineKhaldi AmineKhaldi mentioned this pull request Dec 12, 2024
Open
arvidn
arvidn previously approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

I think both monkeypatch and # type: ignore[assignment] are about equally string hints to the reader that something shady is going on.

@arvidn
Copy link
Contributor

arvidn commented Dec 12, 2024

we still have all the type-unsafety. it's just hidden in monkeypatch.

@arvidn arvidn dismissed their stale review December 13, 2024 13:27

changed my mind

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arvidn arvidn arvidn left review comments

Assignees

@AmineKhaldi AmineKhaldi

Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Cleanup Code cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AmineKhaldi @arvidn