-
Notifications
You must be signed in to change notification settings - Fork 28
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
#2142. Add syntax tests #2148
#2142. Add syntax tests #2148
Conversation
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 one thing I believe must be changed: ET.new(42)
is not an error when ET
declares a constructor (a primary constructor in this case, but any constructor would do) whose name is ET
.
I also mentioned a couple of places where a test will need to be adjusted in the future if we adopt the primary constructor proposal (in its current form or something that isn't too different). I don't know what's the best approach; perhaps we can just ignore this situation and rely on our ability to rediscover it when needed? .. another approach could be to put a TODO
comment saying that "when we have primary constructors, this needs to change as follows: ...". WDYT?
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.
Updated. Added TODO
for the tests that may be affected by the primary constructors
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
2023-07-28 [email protected] dart-lang/co19#2142. Fix typo (dart-lang/co19#2155) 2023-07-26 [email protected] dart-lang/co19#2149. Fix new errors due to changes with the constant evaluator (dart-lang/co19#2152) 2023-07-26 [email protected] dart-lang/co19#2145. Add more tests for Variable (dart-lang/co19#2146) 2023-07-25 [email protected] dart-lang/co19#2142. Add syntax tests (dart-lang/co19#2148) Change-Id: I6ee222b74fc092d93a1b76d35d89a008a97056be Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316590 Commit-Queue: William Hesse <[email protected]> Reviewed-by: William Hesse <[email protected]>
It turns out that it's easier to add new etxtension types tests (which are, in fact, edited copies of inline classes tests) rather then rename and change inline classes ones. I'm going to go on this way and when there will be nothing more to copy and edit I'll delete Inline classes folder