-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[tool] add an skip-confirmation
flag to publish-package for running the command on ci
#3842
Conversation
@@ -84,6 +84,14 @@ class PublishPluginCommand extends PluginCommand { | |||
defaultsTo: false, | |||
negatable: true, | |||
); | |||
argParser.addFlag(_yesOption, | |||
help: 'Running the command without asking for Y/N inputs.\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Running/Run/
help: 'Running the command without asking for Y/N inputs.\n' | ||
'This command will add a `--force` flag to the `pub publish` command if it is not added with $_pubFlagsOption\n' | ||
'It also skips the y/n inputs when pushing tags to remote.\n' | ||
'If running this on CI, an environment variable named $_pubCredentialName must be set to a String that represents the pub credential JSON.\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This (and the next line) isn't really about this flag in particular, is it?
@@ -93,6 +101,9 @@ class PublishPluginCommand extends PluginCommand { | |||
static const String _remoteOption = 'remote'; | |||
static const String _allChangedFlag = 'all-changed'; | |||
static const String _dryRunFlag = 'dry-run'; | |||
static const String _yesOption = 'yes'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about --skip-confirmation
since that makes it self-documenting where it's used?
(Also because no-skip-confirmation
makes a lot more sense for the negated version than no-yes
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartmorgan Updated per comments! PTAL
yes
flag to publish-package for running the command on ciskip-confirmation
flag to publish-package for running the command on ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits.
fileSystem.file(credentialPath) | ||
..createSync(recursive: true) | ||
..writeAsStringSync('some credential'); | ||
} | ||
|
||
setUp(() async { | ||
// This test uses a local file system instead of an in memory one throughout | ||
// so that git actually works. In setup we initialize a mono repo of plugins | ||
// with one package and commit everything to Git. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move the local filesystem part of this comment up to line 32.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -49,8 +56,7 @@ void main() { | |||
processRunner = TestProcessRunner(); | |||
mockStdin = MockStdin(); | |||
commandRunner = CommandRunner<void>('tester', '') | |||
..addCommand(PublishPluginCommand( | |||
mockPackagesDir, mockPackagesDir.fileSystem, | |||
..addCommand(PublishPluginCommand(mockPackagesDir, fileSystem, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to move away from mockPackagesDir
, since it's weirdly indirect. parentDir
is the same directory, but much more clear based on local context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
… running the command on ci (flutter/plugins#3842)
… running the command on ci (flutter/plugins#3842)
… running the command on ci (flutter/plugins#3842)
… running the command on ci (flutter/plugins#3842)
… running the command on ci (flutter/plugins#3842)
… running the command on ci (flutter/plugins#3842)
… running the command on ci (flutter/plugins#3842)
… running the command on ci (flutter/plugins#3842)
… the command on ci (flutter#3842) the skip-confirmation flag will add a --force flag to pub publish, it will also let users to skip the y/n question when pushing tags to remote. Fixes flutter/flutter#79830
the
skip-confirmation
flag will add a--force
flag topub publish
, it will also let users to skip the y/n question when pushing tags to remote.Fixes flutter/flutter#79830
Pre-launch Checklist
dart format
. See plugin_tool format)[shared_preferences]
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.