[flutter_plugin_tools] Complete migration to NNBD #4048
Conversation
| final String? remoteUrl = await _verifyRemote(remoteName); | ||
| if (remoteUrl == null) { | ||
| printError( | ||
| 'Unable to find URL for remote $remoteName; cannot push tags'); | ||
| throw ToolExit(1); | ||
| } |
There was a problem hiding this comment.
nits: Can refactor this into _RemoteInfo()
There was a problem hiding this comment.
I think putting very non-trivial logic (running an external process, potentially killing the whole execution) in the constructor of a data class would be much less clear; keeping it as a simple data object with the logic outside it makes the object, and the failure cases, easier to understand IMO.
| @@ -408,15 +403,15 @@ Safe to ignore if the package is deleted in this commit. | |||
| return statusOutput.isEmpty; | |||
| } | |||
|
|
|||
| Future<String> _verifyRemote(String remote) async { | |||
| Future<String?> _verifyRemote(String remote) async { | |||
| final io.ProcessResult remoteInfo = await processRunner.run( | |||
There was a problem hiding this comment.
nits: Maybe have a different name here as there is a RemoteInfo class now and it might be confusing.
maybe getRemoteUrlResult?
| return _credentialsPath; | ||
| } | ||
| } | ||
|
|
||
| /// The path in which pub expects to find its credentials file. | ||
| final String _credentialsPath = () { | ||
| final String? _credentialsPath = () { |
There was a problem hiding this comment.
nits:
maybe we can move the above logic in _ensureValidPubCredential:
final String? credentialsPath = _credentialsPath;
if (credentialsPath == null) {
printError('Unable to determine pub credentials path');
throw ToolExit(1);
}
here, so _credentialsPath can return a non-null String, then static String? getCredentialPath() can also return a non-null String
There was a problem hiding this comment.
Good thought, done.
stuartmorgan
added
the
waiting for tree to go green
|
This pull request is not suitable for automatic merging in its current state.
|
fluttergithubbot
removed
the
waiting for tree to go green
| @@ -500,15 +494,16 @@ Safe to ignore if the package is deleted in this commit. | |||
| } | |||
|
|
|||
| void _ensureValidPubCredential() { | |||
| final String credentialsPath = _credentialsPath; | |||
| final File credentialFile = packagesDir.fileSystem.file(_credentialsPath); | |||
There was a problem hiding this comment.
nits:
| final File credentialFile = packagesDir.fileSystem.file(_credentialsPath); | |
| final File credentialFile = packagesDir.fileSystem.file(credentialsPath); |
stuartmorgan
added
the
waiting for tree to go green
Migrates the publish command to NNBD.
This is the last command, so the top-level files are migrated as well, allowing the tool to run in strong mode.
Fixes flutter/flutter#81912
Pre-launch Checklist
dart format.)[shared_preferences]///).