Skip to content
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

New use cases for Factory Constructor #2584

Merged
merged 9 commits into from
Aug 19, 2020

Conversation

stupendoussuperpowers
Copy link
Contributor

Attempt to resolve #2202

@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Aug 17, 2020
@kwalrath
Copy link
Contributor

Nice work, @stupendoussuperpowers! I'm going to let @rkj do the technical review of the text, since they filed the original bug. Once that's done, I can help you get the build working and maybe tweak the text a bit.

@kwalrath kwalrath self-requested a review August 17, 2020 23:29
@kwalrath
Copy link
Contributor

@rkj says this looks great to him, so I'll aim to fix the build tomorrow.

@stupendoussuperpowers
Copy link
Contributor Author

Let me know if there's anything I can help out with!

@kwalrath
Copy link
Contributor

I think I fixed the issues with the example (please take a look at the diffs), but the build is still failing for some reason. The issue is in two files not edited in this PR (see below).

@theacodes do you have a theory as to why this PR is failing analysis when the main build is fine?

   error • The argument type 'int' can't be assigned to the parameter type 'String'. • lib/library_tour/core/collections.dart:5:14 • argument_type_not_assignable
  error • A value of type 'dynamic' can't be assigned to a variable of type 'Person'. • lib/library_tour/core/hash_code.dart:24:21 • invalid_assignment

@stupendoussuperpowers
Copy link
Contributor Author

I checked out the diffs, kind of embarassed about that mistake in the code.

As far as the files are concerned, not sure how useful my little detective work is but:

This is the code for the file /hash_code.dart, which has documentation which implies that we are supposed to ignore the invalid_assigment error that we saw above?

if (other is! Person) return false;  
// ignore: stable, dev, invalid_assignment, // https://github.com/dart-lang/sdk/issues/32236  
Person person = other;  

As far as the collections.dart is concerned, the error is pretty straightforward, we are storing an int in a List

Is there any chance that these errors popped up during the last build as well? And something changed in how these files are being loaded/deployed?

@theacodes
Copy link
Contributor

theacodes commented Aug 19, 2020

Build is green now, just needed to update a string in an affected test.

Should be good to go now. The beta build is failing because of an unrelated analysis change, but I can send a follow-up PR to fix that.

@kwalrath
Copy link
Contributor

Oops, I forgot to update that test. :(

@kwalrath
Copy link
Contributor

Thank you, @theacodes! I was so mystified by the failure message that I missed the obvious.

@stupendoussuperpowers are you OK with my code and text changes? I've staged them:

https://kw-www-dartlang-1.firebaseapp.com/guides/language/language-tour#factory-constructors

@stupendoussuperpowers
Copy link
Contributor Author

Yes, I went over the changes and I am fine with them

@theacodes
Copy link
Contributor

theacodes commented Aug 19, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Contributor has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mention initializing final variables using factory constructors
4 participants