-
Notifications
You must be signed in to change notification settings - Fork 2
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
Content-Type header fixes to HTTP.RequestBytes #34
base: main
Are you sure you want to change the base?
Conversation
… a GET has content for example
WalkthroughThe recent updates to Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
Frends.HTTP.RequestBytes/CHANGELOG.md (1)
8-8
: Fix repeated word in changelog.There is a repeated word "Fixed" in the changelog entry for version 1.0.1.
- Fixed Code Coverage + Code Coverage improvementsTools
LanguageTool
[duplication] ~8-~8: Possible typo: you repeated a word
Context: ... requests. ## [1.0.1] - 2024-01-17 ### Fixed - Fixed issues which CodeQL found in the codeba...(ENGLISH_WORD_REPEAT_RULE)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- Frends.HTTP.RequestBytes/CHANGELOG.md (1 hunks)
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.Tests/UnitTests.cs (1 hunks)
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.csproj (1 hunks)
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes/RequestBytes.cs (2 hunks)
Files skipped from review due to trivial changes (1)
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.csproj
Additional context used
LanguageTool
Frends.HTTP.RequestBytes/CHANGELOG.md
[duplication] ~8-~8: Possible typo: you repeated a word
Context: ... requests. ## [1.0.1] - 2024-01-17 ### Fixed - Fixed issues which CodeQL found in the codeba...(ENGLISH_WORD_REPEAT_RULE)
Additional comments not posted (2)
Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes/RequestBytes.cs (1)
156-156
: Improved flexibility in HTTP request handling.The removal of the restriction on sending content with specific HTTP methods enhances the flexibility of the HTTP request handling. This change simplifies the logic and aligns with the updated capabilities of the HttpClient.
Ensure that this change does not inadvertently affect any existing functionality that relied on the previous behavior.
Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.Tests/UnitTests.cs (1)
146-165
: Enhanced test coverage withRequestTestGetWithContent
.The addition of the
RequestTestGetWithContent
test method enhances the test coverage by verifying that GET requests can handle content correctly. This test is well-structured and uses a mock HTTP message handler effectively.Ensure that this test covers all expected scenarios for GET requests with content.
Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.csproj
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
Frends.HTTP.RequestBytes/CHANGELOG.md (1)
8-8
: Fix the repeated word issue.There is a repeated word "Fixed" in the changelog entry for version 1.0.1.
- Fixed issues which CodeQL found in the codebase. - Fixed Code Coverage + Fixed issues which CodeQL found in the codebase and improved Code Coverage.Tools
LanguageTool
[duplication] ~8-~8: Possible typo: you repeated a word
Context: ... requests. ## [1.0.1] - 2024-01-17 ### Fixed - Fixed issues which CodeQL found in the codeba...(ENGLISH_WORD_REPEAT_RULE)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- Frends.HTTP.RequestBytes/CHANGELOG.md (1 hunks)
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.Tests/UnitTests.cs (1 hunks)
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.Tests/UnitTests.cs
- Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes/Frends.HTTP.RequestBytes.csproj
Additional context used
LanguageTool
Frends.HTTP.RequestBytes/CHANGELOG.md
[duplication] ~8-~8: Possible typo: you repeated a word
Context: ... requests. ## [1.0.1] - 2024-01-17 ### Fixed - Fixed issues which CodeQL found in the codeba...(ENGLISH_WORD_REPEAT_RULE)
Additional comments not posted (1)
Frends.HTTP.RequestBytes/CHANGELOG.md (1)
3-5
: Changelog entry for version 1.1.0 looks good.The changes are clearly documented, explaining the removal of restrictions on the Content-Type header for HTTP methods. This enhances flexibility and usability.
closes #32
There was handling in place to not allow requests other than POST, PUT, PATCH and DELETE to have content because the HttpClient would fail if it tried to send such requests. This meant that, for example, if the request method was GET, the request was not assigned content, and we only added content headers if
content != null
.It appears that HttpClient has since been updated to allow this kind of requests, I didn't get any expections during local testing, and used httpbin to verify that the headers are sent correctly. Removed the request method check and the SendMethod enum that was used solely for this purpose. Also made it so that content-type headers can be set even if there is no actual content.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores