-
Notifications
You must be signed in to change notification settings - Fork 230
Samples for enhanced enums feature #136
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
enhanced_enums/pubspec.lock
Outdated
@@ -0,0 +1,334 @@ | |||
# Generated by pub |
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.
We shouldn't need this, right?
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.
The lock file in general, you mean?
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.
Right. For teams trying to align their deps it's nice. Otherwise it's a file that just changes randomly and creates a bunch of noise in Git – IMHO
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.
The docs agree with you:
https://dart.dev/guides/libraries/private-files
As a person who arrived at Dart through Flutter, I'm used to checking in the lock file for pretty much everything, whereas you and Parker (as true Dart coders) are probably used to leaving it out more often than including it.
Looks like you're correct, though, so +1 for yanking it. 😄
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.
That guideline is for libraries, and this, unless my reading is incorrect, isn't a library.
My understanding is we check in pubspec.lock
files for executable samples as a way of making it really easy for people to get a working version of the code. dart pub get
will pull a tested set of dependencies that we know work together.
Please revert.
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.
Right now, the convention in this repo is not to commit the lockfiles.
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.
Thanks! I will follow the repo convention then and remove the file.
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 overall, I would consider demonstrating that ==
and hashCode
aren't required and that switch/case can be used with enhanced enums too.
Co-authored-by: John Ryan <[email protected]>
Co-authored-by: John Ryan <[email protected]>
Co-authored-by: John Ryan <[email protected]>
Co-authored-by: John Ryan <[email protected]>
Co-authored-by: John Ryan <[email protected]>
Thanks for the review @johnpryan I applied all the suggestions |
Does this repository not have a CI? My reading of this PR with it's minimum Dart version should fail when Dart stable is run. |
CI won't run until we do the mono_repo dance, I'm okay with merging into main and creating a beta branch if we don't have one already. |
Hi! 👋
I'm doing some work for @RedBrogdon, and creating samples for some new upcoming Dart features.
In this PR, I have created a series of examples for the upcoming enhanced enums feature dart-lang/sdk#47849
I followed the same pattern as in the
extension_methods
examples, creating different files showing different functionality.I also have created a complete example inside the
complete_example.dart
file.All examples also contain tests.