Skip to content

Fix publish cmd - #38490

Merged
mit-mit merged 3 commits into
flutter:masterfrom
mit-mit:fixpublishcmd
Aug 19, 2019
Merged

Fix publish cmd#38490
mit-mit merged 3 commits into
flutter:masterfrom
mit-mit:fixpublishcmd

Conversation

@mit-mit

@mit-mit mit-mit commented Aug 13, 2019

Copy link
Copy Markdown
Member

Fixes #33302
Fixes #38485

Core issues found that this fixes:

  • publish prompts the user and thus needs to run interactively
  • argsparser passes through arguments in .rest. but not flags, so need to define those explicitly

@fluttergithubbot

fluttergithubbot commented Aug 13, 2019

Copy link
Copy Markdown
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 13, 2019
@mit-mit
mit-mit requested a review from jonahwilliams August 13, 2019 21:49
@mit-mit

mit-mit commented Aug 13, 2019

Copy link
Copy Markdown
Member Author

@jonahwilliams I didn't add a test, as I looked at the existing ones and I'm really puzzled how those are passing. There was already a test that runs publish and looks at the output for a y/n. I wonder how that was passing?

@jonahwilliams

jonahwilliams commented Aug 15, 2019

Copy link
Copy Markdown
Contributor

The tests in https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/general.shard/commands/packages_test.dart#L348 should already cover this, though they need to be adjusted.

These inject a fake process manager which mocks the output/interaction of pub

@codecov

codecov Bot commented Aug 16, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #38490 into master will decrease coverage by 0.41%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #38490      +/-   ##
==========================================
- Coverage   55.91%    55.5%   -0.42%     
==========================================
  Files         194      194              
  Lines       18186    18200      +14     
==========================================
- Hits        10169    10101      -68     
- Misses       8017     8099      +82
Flag Coverage Δ
#flutter_tool 55.5% <80%> (-0.42%) ⬇️
Impacted Files Coverage Δ
...kages/flutter_tools/lib/src/commands/packages.dart 76% <80%> (+0.41%) ⬆️
packages/flutter_tools/lib/src/compile.dart 54% <0%> (-23.35%) ⬇️
...ages/flutter_tools/lib/src/test/test_compiler.dart 54.92% <0%> (-15.5%) ⬇️
...ackages/flutter_tools/lib/src/resident_runner.dart 44.33% <0%> (-9.15%) ⬇️
packages/flutter_tools/lib/src/cache.dart 43.5% <0%> (-0.73%) ⬇️
packages/flutter_tools/lib/src/base/os.dart 29.83% <0%> (+0.8%) ⬆️
packages/flutter_tools/lib/src/dart/pub.dart 77.65% <0%> (+1.06%) ⬆️
packages/flutter_tools/lib/src/base/process.dart 80.46% <0%> (+1.56%) ⬆️
packages/flutter_tools/lib/src/version.dart 90.73% <0%> (+1.95%) ⬆️
...ckages/flutter_tools/lib/src/commands/version.dart 93.33% <0%> (+88.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17ddfb1...d3bc9c6. Read the comment docs.

jonahwilliams
jonahwilliams reviewed Aug 16, 2019
View reviewed changes
Comment thread packages/flutter_tools/test/general.shard/commands/packages_test.dart Outdated
jonahwilliams
jonahwilliams approved these changes Aug 16, 2019
View reviewed changes

@jonahwilliams jonahwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mit-mit Jonah Williams
Fix formatting
d3bc9c6 
Co-Authored-By: Jonah Williams <[email protected]>
@mit-mit
mit-mit merged commit 79d8967 into flutter:master Aug 19, 2019
@mit-mit
mit-mit deleted the fixpublishcmd branch August 19, 2019 07:02
@MaikuB MaikuB mentioned this pull request Sep 17, 2019
Closed
@jmagman jmagman mentioned this pull request Jan 18, 2020
Closed
@jmagman jmagman mentioned this pull request Mar 13, 2020
Closed
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@jonahwilliams jonahwilliams jonahwilliams approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

flutter pub publish fails to publish flutter pub publish --dry-run shows a "Could not find an option named "dry-run" error

4 participants

@mit-mit @fluttergithubbot @jonahwilliams @googlebot