Skip to content

Conversation

sansyrox
Copy link
Member

@sansyrox sansyrox commented Sep 6, 2025

Description

This PR fixes #

Summary

This PR does....

PR Checklist

Please ensure that:

  • The PR contains a descriptive title
  • The PR contains a descriptive summary of the changes
  • You build and test your changes before submitting a PR.
  • You have added relevant documentation
  • You have added relevant tests. We prefer integration tests wherever possible

Pre-Commit Instructions:

Review by RecurseML

🔍 Review performed on ffb7ebc..83e1ed5

Severity Location Issue Action
Low integration_tests/test_request_json.py:33 Redundant comment that restates what the code does Dismiss
Low integration_tests/test_request_json.py:50 Redundant comment that restates what the code does Dismiss
Low integration_tests/test_request_json.py:90 Redundant comment that restates what the code does Dismiss
Low integration_tests/test_request_json.py:92 Redundant comment that restates what the code does Dismiss
Low integration_tests/test_request_json.py:95 Redundant comment that restates what the code does Dismiss
✅ Files analyzed, no issues (2)

integration_tests/base_routes.py
src/types/request.rs

Analyze latest changes

Need help? Join our Discord

Copy link

vercel bot commented Sep 6, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
robyn Error Error Sep 6, 2025 1:28pm

assert response.status_code == 200
result = response.json()

# Check that the array was parsed as a list
Copy link

Choose a reason for hiding this comment

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

This comment simply restates what the next line of code does (assert result["type"] == "list"). According to the effective comments rule, comments should not state the obvious or merely restate what the code is doing. The assertion itself is clear enough to understand without this redundant comment.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
# Check that the array was parsed as a list

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

assert response.status_code == 200
result = response.json()

# Check that the object was parsed as a dict
Copy link

Choose a reason for hiding this comment

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

This comment simply restates what the next line of code does (assert result["type"] == "dict"). According to the effective comments rule, comments should not state the obvious or merely restate what the code is doing. The assertion itself clearly shows we're checking if the object was parsed as a dict.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
# Check that the object was parsed as a dict

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

result = response.json()

# Check type
assert result["type"] == expected_type, f"Expected {expected_type} but got {result['type']} for data {test_data}"
Copy link

Choose a reason for hiding this comment

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

This comment is redundant as it merely states what the next line of code does (checking the type with an assertion). The assertion itself clearly indicates we're checking the type, making the comment unnecessary according to the effective comments rule.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

# Check type
assert result["type"] == expected_type, f"Expected {expected_type} but got {result['type']} for data {test_data}"

# Check data integrity
Copy link

Choose a reason for hiding this comment

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

This comment restates what the next line of code does (assert result["data"] == test_data). The assertion itself clearly shows we're checking data integrity by comparing the result data with the test data. According to the effective comments rule, comments should not merely restate what is obvious from the code.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
# Check data integrity

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

# Check data integrity
assert result["data"] == test_data, f"Data integrity failed for {test_data}"

# Check specific type flags
Copy link

Choose a reason for hiding this comment

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

This comment simply describes what the following loop is doing, which is already clear from the code itself. The loop iterates through expected_checks and verifies each one with assertions. According to the effective comments rule, comments should not restate what is obvious from reading the code.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
# Check specific type flags

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

Copy link

codspeed-hq bot commented Sep 6, 2025

CodSpeed Performance Report

Merging #1243 will not alter performance

Comparing fix/unsupported-json-types (400aedb) with main (a77063a)1

Summary

✅ 150 untouched benchmarks

Footnotes

  1. No successful run was found on main (ffb7ebc) during the generation of this report, so a77063a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

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.

1 participant