Conversation
inline lints directly in analysis_options.yaml Signed-off-by: mark <mark@dropbear.dev>
Signed-off-by: mark <mark@dropbear.dev>
Signed-off-by: mark <mark@dropbear.dev>
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 delivers a significant overhaul of the Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a complete rewrite of the package, transitioning from a custom parser to a more maintainable implementation that wraps Dart's native Uri class. This is a significant architectural improvement. The update also includes stricter linting rules, comprehensive documentation, and an expanded test suite. My review identified one critical issue in the new Iri class implementation regarding the toUriString() method, which I've detailed in a specific comment.
Signed-off-by: mark <mark@dropbear.dev>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is an impressive and substantial rewrite of the package. The new implementation, which wraps the native Uri class, is a significant improvement in terms of simplicity, maintainability, and readability over the previous custom parser. The API is now more familiar to Dart developers, and the addition of comprehensive tests and clear documentation (including limitations) is commendable. I've identified a minor bug in the normalization of query parameters when they contain iterable values, and I've provided suggestions for a fix.
Ensures that values within are correctly normalized using NFKC. Previously, only values were normalized, leading to potential issues with international characters in multi-valued query parameters. This change applies to constructors (, , ) and the method. A new test case has been added to verify this behavior. Signed-off-by: mark <mark@dropbear.dev>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a complete rewrite of the package, moving from a custom IRI parser to a wrapper around Dart's native Uri class. This is a significant improvement in terms of simplicity, robustness, and maintainability. The new API is clean and mirrors the Uri API, making it familiar to Dart developers. The use of unorm_dart for Unicode normalization is a great choice.
I've added a couple of suggestions for minor improvements, mainly around code duplication and a potential performance optimization by caching the result of toUri(). Overall, this is an excellent refactoring.
Extracts duplicated query parameter normalization logic into a private static method `_normalizeQueryParameters` to improve code maintainability and readability. Adds a new `queryParametersAll` getter to the `Iri` class, providing a way to access all query parameters, including those with multiple values per key. Enhances the test suite with comprehensive test cases for various `queryParameters` scenarios, including `Iterable<String>` values and international characters, across different constructors (`Iri`, `Iri.http`, `Iri.https`) and the `replace` method. These tests now utilize the new `queryParametersAll` getter for robust verification. Signed-off-by: mark <mark@dropbear.dev>
Optimizes the `toUri()` method by introducing a `late final _convertedUri` field to cache its result. This prevents redundant computations, especially when `toUri()` is called multiple times by `hashCode`, `operator==`, or `toUriString()` for the same immutable `Iri` instance. The original logic of `toUri()` has been moved to a new private method, `_computeUri()`, which is called only once upon first access to the cached field. This improves performance, particularly when `Iri` objects are used in collections. Signed-off-by: mark <mark@dropbear.dev>
Signed-off-by: mark <mark@dropbear.dev>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a complete rewrite of the iri package, moving from a custom parser to a wrapper around Dart's native Uri class. This is a significant improvement in terms of simplicity, maintainability, and leveraging the battle-tested Uri implementation. The new API is clean and mirrors the Uri API, which is great for usability. The addition of stricter linting rules and comprehensive tests, including skipped tests for known limitations, is commendable. I've found a few minor issues in documentation comments and a more significant issue with how query parameters are handled, which I've detailed in my review comments. Overall, this is an excellent update.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: mark-dropbear <76698672+mark-dropbear@users.noreply.github.com>
1 participant
No description provided.