Skip to content
This repository was archived by the owner on Nov 1, 2024. It is now read-only.

Conversation

@natebosch
Copy link
Contributor

The field in AnsiProgresshad been non-final so that it could be
initialized in the constructor, the initialization expression needs
access to this. With null safety this pattern can be expressed with a
late final field.

The field in VerboseLogger could have been final before the
migration.

Append -dev to the version since this can't be published until it is
rolled and validated internally.

The field in `AnsiProgress`had been non-final so that it could be
initialized in the constructor, the initialization expression needs
access to `this`. With null safety this pattern can be expressed with a
`late final` field.

The field in `VerboseLogger` could have been `final` before the
migration.

Append `-dev` to the version since this can't be published until it is
rolled and validated internally.
@natebosch natebosch requested review from jakemac53 and pq November 9, 2020 21:56
@google-cla google-cla bot added the cla: yes label Nov 9, 2020
Copy link
Contributor

@pq pq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, nice!

I didn't know about this pattern. I'll have to read-up.

Thanks!

pubspec.yaml Outdated
@@ -1,5 +1,5 @@
name: cli_util
version: 0.3.0-nullsafety.0
version: 0.3.0-nullsafety.0-dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the goal is to publish this once it rolls through the sdk etc, so we likely don't need the -dev here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we could also drop the -dev and publish after the role - we wouldn't need to wait for a subsequent sync for a changelog and pubspec change, and it gives us more of a signal to verify things.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya I just question if its worth doing this for every single package or not?

Not sure what precedent we want to set here, I can see it either way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that our existing guidelines do not say to do this (they suggest doing what @pq already did and just waiting to publish until the internal validation happens)

Optimistically we'll publish right after validation.
@natebosch natebosch merged commit 2fbb4eb into master Nov 9, 2020
@natebosch natebosch deleted the late-final-field branch November 9, 2020 23:51
mosuem pushed a commit to dart-lang/tools that referenced this pull request Oct 25, 2024
The field in `AnsiProgress`had been non-final so that it could be
initialized in the constructor, the initialization expression needs
access to `this`. With null safety this pattern can be expressed with a
`late final` field.

The field in `VerboseLogger` could have been `final` before the
migration.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

4 participants