-
Notifications
You must be signed in to change notification settings - Fork 29
#3315. Add tests for formal parameters. Part 5. #3348
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
Conversation
eernstg
left a comment
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.
Looks good! Just two nit-picking comments.
| } | ||
|
|
||
| class C2<T> { | ||
| this({var String v1 = "", final T? v2, var bool v3 = true}); |
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.
Perhaps use final/var/final in this second test, just like in the previous library.
| } | ||
|
|
||
| class C2<T> { | ||
| this({required var int v1, required final T v2, required var bool v3}); |
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.
Same issue here, using final/var/final rather than var/final/var in the second test seems to give slightly better coverage, so we might as well do that.
|
Updated. PTAL. |
eernstg
left a comment
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.
LGTM
2025-10-16 sgrekhov22@gmail.com dart-lang/co19#3315. Add tests for super parameters (dart-lang/co19#3352) 2025-10-16 sgrekhov22@gmail.com dart-lang/co19#3315. Add more declaring constructors tests (dart-lang/co19#3351) 2025-10-15 sgrekhov22@gmail.com dart-lang/co19#3315. Add tests for initializer list (dart-lang/co19#3349) 2025-10-15 sgrekhov22@gmail.com dart-lang/co19#3315. Add tests for the `new` keyword (dart-lang/co19#3350) 2025-10-14 sgrekhov22@gmail.com dart-lang/co19#3315. Add tests for formal parameters. Part 5. (dart-lang/co19#3348) 2025-10-13 sgrekhov22@gmail.com dart-lang/co19#3315. Update assertions. Add test for setters. (dart-lang/co19#3347) 2025-10-10 sgrekhov22@gmail.com dart-lang/co19#3315. Add tests for formal parameters. Part 4. (dart-lang/co19#3346) 2025-10-10 sgrekhov22@gmail.com dart-lang/co19#3315. Add tests for formal parameters. Part 3. (dart-lang/co19#3345) 2025-10-10 sgrekhov22@gmail.com dart-lang/co19#3315. Add tests for formal parameters. Part 2. (dart-lang/co19#3344) R=athom@google.com, eernst@google.com Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try,dart2js-minified-linux-d8-try Change-Id: Ie4824457e4c1843e8406dbb7ba8c0a6a10728410 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/455600 Auto-Submit: Sergey Grekhov <sgrekhov22@gmail.com> Reviewed-by: Alexander Thomas <athom@google.com> Reviewed-by: Erik Ernst <eernst@google.com> Commit-Queue: Alexander Thomas <athom@google.com>
No description provided.