[flutter_plugin_tools] Complete migration to NNBD#4048
[flutter_plugin_tools] Complete migration to NNBD#4048fluttergithubbot merged 5 commits intoflutter:masterfrom
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.
|
|
||
| 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?
|
|
||
| /// 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-g
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
cyanglaz
left a comment
cyanglaz
left a comment
There was a problem hiding this comment.
Still LGTM % nits. CI failure should be an easy fix too.
|
|
||
| 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-g
added
the
waiting for tree to go green
3 participants
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]///).