Migrate the Animations Package to NNBD

[pennzht]

This PR migrates the animations package to NNBD.

Thank you very much!

Currently blocked on:

• Linter rule avoid_as: cannot find good alternative. https://dart-lang.github.io/linter/lints/avoid_as.html

[pennzht]

I still cannot make the "publishable" check pass. What should I do?

[pennzht]

@goderbauer@gspencergoog@shihaohong

Hi! How is everything going?

I'm just wondering:

• When can we merge this?
• When can we publish this?

[shihaohong]

I think Michael and Greg are out until next week (it's Thankgiving week in the US). I don't think either of them commented on publishing and merging the NNBD version of the animations package and while I'm on board, I want to make sure we iron out any versioning weirdness that could happen on GitHub and pub.

So far, I've come up with two scenarios that could occur, but neither seem to raise any serious red flags for me:

1. NNBD prerelease is published (2.0.0-nullsafety.0).
2. Hypothetically, some new packages/animations features are added and someone unknowingly tries to publish the next version before NNBD reaches flutter/stable:

• Publishing as 1.1.2+x: On packages/master we're on 1.1.2 and someone tries publishing new features/fixes on master. This probably will not cause any issues, but I'm not sure what would actually happen (like pub disallowing publishing versions older than the latest version), since I've never tried this.
• Publishing as 2.0.0-nullsafety.0+x: Someone could unknowingly try to publish a 2.0.0-nullsafety.0+x from packages/master, which would be bad since it wouldn't actually contain the NNBD migration. However, we could just publish another prerelease and increment the prerelease version number on top of it to correct this.

Edit: I just thought of another potential scenario -- If there's some "hotfix" necessary for the animations package, we might not be able to fix it until the stable NNBD migration for the package is complete. I don't think that this is likely, but I wanted to just put that out there.

So basically, if we do make publish a prerelease, we need to be clear that new PRs for packages/animations are not incorrectly published.

Short Term (before flutter/stable has NNBD)

1. We're merging the NNBD migration to a packages/nnbd branch instead of packages/master. Edit: Dockerfile for packages/nnbd also has to be updated to use the Flutter beta branch for CI checks.
2. We'll publish packages/animations from nnbd as 2.0.0-nullsafety.x. Any unexpected bugs in the NNBD version should be fixed in the nnbd branch.

Longer Term (once flutter/stable is NNBD)

1. Make sure packages/animations is not published beyond 2.0.0-nullsafety.0 from the master branch until the NNBD changes are properly merged into packages/animations on the master branch.
2. Once flutter/flutter is NNBD stable, we merge the nnbd branch into the master branch and publish a stable 2.0.0 release. Edit: Make sure packages/master Dockerfile uses Flutter stable branch for CI checks and not the beta branch.

cc/ @HansMuller, who might be able to provide some feedback while Greg and Michael are out on what he thinks about this process.

[shihaohong]

Nice, thanks for removing this!

[pennzht]

@shihaohong

Thank you for your message! Overall this sounds good to me, as long as we can publish 1.1.2+x after we publish 2.0.0-nullsafety.0

The 2-branch model is currently being used in some packages such as crypto (see https://github.com/dart-lang/crypto/tree/3.0 for a null-safe branch).

I think it's fine to publish newer prerelease versions for the null-safe branch, such as 2.0.0-nullsafety.1, as long as it does not reach 2.0.0

Also, if animations is a frequently published package, it's probably a good idea to keep the main and null-safe branches in sync regularly.

[shihaohong]

We don't publish animations very frequently, and the number of PRs we've been getting have been pretty low so I think we should be good with the plan as long as we're proactive with merging nnbd to master once flutter/stable reaches NNBD :)

[shihaohong]

@pennzhtcrypto's 3.0 null-safe branch isn't published right, even as a prerelease? Would that be the main difference between how we're handling crypto and animations?

[pennzht]

@shihaohong

@pennzhtcrypto's 3.0 null-safe branch isn't published right, even as a prerelease? Would that be the main difference between how we're handling crypto and animations?

Correct, crypto's null-safe version has not been published yet, even as a prerelease.

Publishinganimations is not very urgent at the moment; your comment #245 (comment) sounds good to me.

Having a migrated version available, even in a separate nnbd branch, is rather urgent, since we need to unblock Motion Codelab and Gallery, both of which depend on animations.
Even if animations is not published, we can still use a git reference for the moment.

Would it be fine to merge this PR? The CI is currently blocking, but I don't know how to resolve it.

Thank you!

[pennzht]

@rami-a

[shihaohong]

@pennzhtI just figured it out! I figured out part of it. We need to update the CI to use the dev or beta branch of Flutter for the CI checked in the nnbd branch. Here is what I got so far.

On top of that, I'm running into some issues generating the new Docker image. Somehow, installing clang-format-5.0 is causing some hassle.

Also, since it's Thanksgiving holidays for the US, we may not hear from the rest of the team until the start of next week at the earliest. I think goderbauer is also out for next week as well.

[pennzht]

@shihaohong

Thank you so much for helping us on this migration! Please let us know how it goes.
Also, if you need anything from us, please let us know too.

[shihaohong]

The CI modifications were made in #249 and that should unblock the CI checks once this branch has been merged with the latest nnbd branch.

[goderbauer]

flutter/flutter#70726 has landed. @pennzht can you rebase this PR on top of the latest changes, which probably means removing this !.

[goderbauer]

It is odd that this field is nullable.

[rami-a]

It has to be nullable since the radio button's onChanged callback is final ValueChanged<T?>? onChanged

[rami-a]

Closing this since #253 was merged and is the same thing