Skip to content
This repository has been archived by the owner on Nov 5, 2022. It is now read-only.

migrate to null safety #50

Merged
merged 2 commits into from Apr 10, 2020
Merged

migrate to null safety #50

merged 2 commits into from Apr 10, 2020

Conversation

jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented Apr 10, 2020

I am not sure what the unawaited_futures lint would do with a Future? type. If it triggers we may want to allow Future<void>? as the argument here.

I went ahead and made the api nullable, at least with the current lint implementation it does trigger and I think it makes sense to.

I also added an example since I needed to do that to test the lint behavior anyways.

@jakemac53 jakemac53 changed the title opt into null safety migrate to null safety Apr 10, 2020
@jakemac53 jakemac53 merged commit 0da6406 into null_safety Apr 10, 2020
@jakemac53 jakemac53 deleted the null_safety_migration branch April 10, 2020 16:32
@jakemac53
Copy link
Contributor Author

crap I did a normal merge lol, oh well

@davidmorgan
Copy link
Contributor

Hi Jake, hi Nate,

Would it be possible to leave a route for us to release new lint lists for pre-NNBD? It seems pretty likely we're going to want to release at least one or two versions before NNBD is stable.

I guess, we can do parallel releases for pre-NNBD and post, with the yaml files the same between the two and the code having the changes in this PR.

The only difficult point is versioning, we name the yaml files after the released version and can't name it after both NNBD and pre-NNBD versions.

How about we use 1.x.0 for pre-NNBD releases and 1.x.1 for NNBD releases? That way each NNBD release has a higher version number than the corresonding pre-NNBD release, and the minor version still matches the yaml file.

That would make this change 1.9.1, indicating that it adds NNBD support but doesn't come with a new yaml file.

WDYT?

Thanks!

@jakemac53
Copy link
Contributor Author

Note that this is only a branch right now - so its just for doing git overrides with. We can continue development on the master branch and merge those into this branch.

@davidmorgan do you want to explicitly suggest different lints with and without NNBD? Or is it that you want to have a version that doesn't have such a high min sdk constraint?

If we want to release separate NNBD versions I would suggest doing x.x.x-nnbd or something like that.

@davidmorgan
Copy link
Contributor

Ah, I was thinking people not using NNBD would be unable to use the new version, but of course that's not true: they have to meet the new min version constraint but they don't have to opt into NNBD.

I guess I'm okay once the min version constraint is a stable version. If I understand correctly that should be ~soon, so it will likely be before I want to release a new package_pedantic. Does that sound right?

No need for different lints for NNBD :)

Thanks!

@jakemac53
Copy link
Contributor Author

The min sdk constraint won't be able to be a stable version for a while I don't think (experiments aren't usable in stable SDKs), but we may want to publish something before then. However, those should only be pre-release versions if they require an experiment to be enabled in order to work.

I propose that we publish 1.x.x and 1.x.x-nnbd. These would be identical except for the nnbd changes (including restricted sdk constraints). Only dev channel users would be allowed to get the -nnbd version (based on the sdk constraint we set), and they would only get it if explicitly specified as well since pub would prefer the stable version to the prerelease versions.

description: >-
The Dart analyzer settings and best practices used internally at Google.
homepage: https://github.com/dart-lang/pedantic

environment:
sdk: '>=2.1.1-dev.0.0 <3.0.0'
sdk: '>=2.8.0-dev <3.0.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should change this to <2.8.0 so that only 2.8.-dev sdks can select this release, per other comments.

@natebosch
Copy link
Contributor

I propose that we publish 1.x.x and 1.x.x-nnbd. These would be identical except for the nnbd changes (including restricted sdk constraints).

I would strongly prefer not to be publishing to pub from different branches. My preference would be to keep the null_safety branch until the point we need to publish to pub. At that point we should merge to master, publish from there, and not worry about publishing any non-nnbd versions anymore, unless there is an emergency situation where we can go back and fork.

@jakemac53
Copy link
Contributor Author

I think pre-release versions are an acceptable exception to the general rule of publishing from master.

@davidmorgan
Copy link
Contributor

pedantic is mostly about publishing yaml files, not code; so it's not really an emergency situation, it's expected that we want to be able to publish yaml files every two or three months in a form most people can use.

I'm happy with any way we can figure out to do that ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants