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.

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

ratelimits: Update errors to deep link to individual limits documentation #7767

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

beautifulentropy
Copy link
Member

@beautifulentropy beautifulentropy commented Oct 23, 2024

Updates rate limits error messages to deep link to new website docs added in letsencrypt/website#1756.


Reviewers, please check the links against the preview of these changes.

@beautifulentropy beautifulentropy changed the title WIP ratelimits: Deep link to individual limits documentation Oct 23, 2024
beautifulentropy added a commit to letsencrypt/website that referenced this pull request Oct 23, 2024
#1755)

Reverts #1754

Reverting these docs updates until the old URLs replaced in
letsencrypt/boulder#7767 are deployed to
production.
@beautifulentropy beautifulentropy marked this pull request as ready for review October 24, 2024 15:00
@beautifulentropy beautifulentropy requested a review from a team as a code owner October 24, 2024 15:00
@beautifulentropy beautifulentropy changed the title ratelimits: Deep link to individual limits documentation ratelimits: Update errors to deep link to individual limits documentation Oct 24, 2024
@@ -583,7 +599,7 @@ func TestRateLimitError(t *testing.T) {
test.AssertNotError(t, err, "expected no error")
} else {
test.AssertError(t, err, "expected an error")
test.AssertContains(t, err.Error(), tc.expectedErr)
test.AssertEquals(t, err.Error(), tc.expectedErr)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't love using AssertEquals for error strings, it makes us a bit too precise imo. I'd prefer that the test case have two fields expectedErr and expectedLink, and assert that the resulting message contains both of them. But that's a super minor thing.

@beautifulentropy beautifulentropy merged commit 6e6c8fe into main Oct 25, 2024
12 checks passed
@beautifulentropy beautifulentropy deleted the update-limits-docs branch October 25, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants