-
Notifications
You must be signed in to change notification settings - Fork 53
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
Fix compile error introduced by #44 #47
Conversation
LogWhen running the code:
When running old workable code:
|
98d78d3
to
88c9f2b
Compare
@creativecreatorormaybenot Could you please have a try on this PR and see whether the error is related to your tests? B/c seems that you are using a newer Flutter version, so Flutter behavior may change because of that. |
All right. I see the error comes from this PR instead of @creativecreatorormaybenot's code on stable. Run @creativecreatorormaybenot 's code in beta looks good:
But running this PR on beta has errors:
|
@creativecreatorormaybenot I have not found clue why this fails... Could you please have a look at it as well? Sounds like the follower layer ends up before the leader layer. |
All right. Now tests pass on beta channel.
And the stable channel:
|
Lesson: Flutter beta's code is very, very different from Flutter stable's code. |
@creativecreatorormaybenot Btw, the experience above shows that, even if we make that cherry pick, Flutter Portal still cannot be working on stable branch, until next major release! (I did not expected that) But now it works on stable :) |
@fzyzcjy that is not right. Cherrypicks are included with the next patch - that is the idea behind them. |
@creativecreatorormaybenot Well, from what I have experimented above, the thing is:
So, if that cherry-pick is done, we are in a situation similar to case 4, instead of case 5. In that case, test still errors. In addition I am going to implement new features, and want to based on your great PR :) I just merge it now, and if there are problems, we can discuss and revert if needed. |
@fzyzcjy in that case, I suppose we should be fine as long as you remember to remvoe |
@creativecreatorormaybenot Sure :) |
Or, maybe we should keep it in our source tree. Because, for example, suppose flutter 2.12 (say) is released, but someone is using flutter 2.10 on their computer. Then, if we use the Flutter built-in code, then we are in trouble (case 4 in above). |
We should ignore this case. If they are upgrading a package, we can expect them to run the latest |
@creativecreatorormaybenot Well indeed maybe not expect everyone... I have seen many people using old versions and cannot upgrade due to, e.g. internal changes to frameworks; or they want high stability - you know, flutter 2.x.0 is usually really not that stable IMHO... |
@fzyzcjy I am pretty sure the guideline for packages is sticking to the stable channel. At leat that is what many popular packages do. |
@creativecreatorormaybenot Agree. Then I will do that later. |
#44
/cc @creativecreatorormaybenot