refactor: remove http_methods dependency#513
refactor: remove http_methods dependency#513yuchengpersonal wants to merge 3 commits intodart-lang:masterfrom
Conversation
Replace the external `http_methods` package with an inlined `http_methods.dart` file in `shelf_router`. This removes the dependency on a package not maintained by the Dart team, which is a prerequisite for using `shelf_router` in the Dart SDK. Changes: - Add `pkgs/shelf_router/lib/src/http_methods.dart` with the full IANA HTTP method registry and `isHttpMethod()` function - Update `router.dart` to use the local import - Export `isHttpMethod` from `shelf_router.dart` barrel file - Update `shelf_router_generator` to import from `shelf_router` instead of `http_methods` - Remove `http_methods` from both `pubspec.yaml` files Fixes dart-lang#512 Co-Authored-By: Claude <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively removes the http_methods dependency by inlining its functionality, which is a great step towards reducing external dependencies. The implementation in http_methods.dart is clean and efficient. I've found one minor issue regarding an import path in the generator package that should be adjusted to rely on the public API of shelf_router instead of its internal files. Otherwise, the changes look solid.
| import 'package:build/build.dart' show BuildStep, log; | ||
| import 'package:code_builder/code_builder.dart' as code; | ||
| import 'package:http_methods/http_methods.dart' show isHttpMethod; | ||
| import 'package:shelf_router/src/http_methods.dart' show isHttpMethod; |
There was a problem hiding this comment.
To adhere to best practices, you should avoid importing files from another package's lib/src directory. Since isHttpMethod is now exported from package:shelf_router/shelf_router.dart, it's part of the public API. Please update this import to use the public package URI.
This ensures that shelf_router_generator relies on the stable, public API of shelf_router rather than its internal implementation details, which could change without notice.
| import 'package:shelf_router/src/http_methods.dart' show isHttpMethod; | |
| import 'package:shelf_router/shelf_router.dart' show isHttpMethod; |
Address review feedback from gemini-code-assist: import isHttpMethod from the public package:shelf_router/shelf_router.dart instead of the internal src/http_methods.dart path. This ensures shelf_router_generator relies on the stable, public API.
|
Thanks for the suggestion! I will update the import to use the public API as recommended. |
- Export RouterEntry from shelf_router.dart to allow shelf_router_generator to use public API instead of importing from lib/src - Update shelf_router_generator to import RouterEntry from public API Addresses review feedback from gemini-code-assist
Summary
Remove the external
http_methodspackage dependency and replace it with an inlined implementation, as suggested in #512.Motivation
The
http_methodspackage is not maintained by the Dart team. Removing this external dependency is a prerequisite for potentially usingshelf_routerin the Dart SDK.Changes
pkgs/shelf_router/lib/src/http_methods.dart— contains the full IANA HTTP method registry and theisHttpMethod()functionrouter.dartto use the localhttp_methods.dartimport instead of the external packageshelf_router.dartbarrel file to re-exportisHttpMethodfor downstream consumersshelf_router_generatorto importisHttpMethodfromshelf_routerinstead ofhttp_methodshttp_methodsdependency from bothshelf_router/pubspec.yamlandshelf_router_generator/pubspec.yamlAPI Compatibility
This is a fully backward-compatible change:
isHttpMethod()retains the same signature and behavior (case-insensitive check against IANA methods)httpMethodsconstant set is available for internal useChecklist
shelf_routerandshelf_router_generatorpackagesQUERYfrom RFC-ietf-httpbis-safe-method-w-body-14)Fixes #512
🐙 Generated with OpenClaw