feat: add isOk convenience getter to BaseResponse#1874
feat: add isOk convenience getter to BaseResponse#1874Turskyi wants to merge 3 commits intodart-lang:masterfrom
Conversation
Introduces a boolean getter `isOk` to the `Response` class that returns true if the `statusCode` is 200. This provides a more expressive and readable way for developers to verify successful responses, reducing the need for magic numbers or platform-specific imports (like dart:io's HttpStatus) in client code.
Add unit tests to verify the behavior of the `isOk` getter on the `Response` class, ensuring it returns `true` only for status code 200.
* Move the `isOk` property from `Response` to `BaseResponse` so it is available to all response types (e.g. `StreamedResponse`). * Update tests to verify `isOk` behavior on both `Response` and `StreamedResponse`.
|
Personally, I'm more in favor of something like an |
Thanks for the suggestion - that makes sense as a broader convenience 👍 Right now this PR adds A broader Happy to update this PR based on maintainer feedback! |
PR HealthLicense Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with Breaking changes ✔️
This check can be disabled by tagging the PR with
Unused Dependencies
|
| Package | Status |
|---|---|
| http | ❗ Show IssuesThese packages may be unused, or you may be using assets from these packages: |
For details on how to fix these, see dependency_validator.
This check can be disabled by tagging the PR with skip-unused-dependencies-check.
Coverage ✔️
| File | Coverage |
|---|---|
| pkgs/http/lib/src/base_response.dart | 💚 88 % ⬆️ 1 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
Changelog Entry ❗
| Package | Changed Files |
|---|---|
| package:http | pkgs/http/lib/src/base_response.dart |
Changes to files need to be accounted for in their respective changelogs.
This check can be disabled by tagging the PR with skip-changelog-check.
|
I don't think that we can merge this as-is because there could be instances of Instead, we could implement this as an extension method. I'd suggest one check per response type, e.g. WDYT? |
This PR adds a convenience getter isOk to the BaseResponse class.
What is changing? I have added a boolean getter isOk to BaseResponse that returns true if the statusCode is exactly 200. By placing it in the base class, this convenience is now available for all response types, including Response and StreamedResponse.
I have also added unit tests to verify this behavior across both response types and various status codes (200, 201, 404, 500).
Why? Checking for a 200 OK response is the most frequent operation when handling HTTP requests. Currently, developers must either:
This addition makes the API more intuitive and readable while remaining platform-agnostic:
•
[x] I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:
• See our contributor guide for general expectations for PRs. • Larger or significant changes should be discussed in an issue before creating a PR. • Contributions to our repos should follow the Dart style guide and use dart format. • Most changes should add an entry to the changelog and may need to rev the pubspec package version. • Changes to packages require corresponding tests. Many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.