Skip to content

Adds list required components when VS is not installed#40397

Merged
franciscojma86 merged 2 commits intoflutter:masterfrom
franciscojma86:install
Sep 13, 2019
Merged

Adds list required components when VS is not installed#40397
franciscojma86 merged 2 commits intoflutter:masterfrom
franciscojma86:install

Conversation

@franciscojma86
Copy link
Contributor

@franciscojma86 franciscojma86 commented Sep 12, 2019

Description

Adds a list of required components to the user message when Visual Studio is not installed. This list is currently only shown if the user has an incomplete installation.

Example of the message:

[X] Visual Studio - develop for Windows
    X Visual Studio not installed; this is necessary for Windows development.
      Download at https://visualstudio.microsoft.com/downloads/.
      Please include the "Desktop development with C++" workload, and the following components:
        MSBuild
        MSVC v142 - VS 2019 C++ x64/x86 build tools (v14.21)
        Windows 10 SDK (10.0.17763.0)

Related Issues

#40267

Tests

Updated visual_studio_validator_test.dart

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@franciscojma86 franciscojma86 added tool Affects the "flutter" command-line tool. See also t: labels. platform-windows Building on or for Windows specifically t: flutter doctor Problem related to the "flutter doctor" tool labels Sep 12, 2019
@franciscojma86 franciscojma86 self-assigned this Sep 12, 2019
@codecov
Copy link

codecov bot commented Sep 12, 2019 •
edited
Loading

Codecov Report

Merging #40397 into master will increase coverage by 0.2%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #40397     +/-   ##
=========================================
+ Coverage   58.78%   58.98%   +0.2%     
=========================================
  Files         192      192             
  Lines       18596    18599      +3     
=========================================
+ Hits        10932    10971     +39     
+ Misses       7664     7628     -36
Flag Coverage Δ
#flutter_tool 58.98% <100%> (+0.2%) ⬆️
Impacted Files Coverage Δ
...tools/lib/src/windows/visual_studio_validator.dart 93.33% <100%> (+0.74%) ⬆️
...ages/flutter_tools/lib/src/base/user_messages.dart 51.35% <100%> (+0.44%) ⬆️
...ckages/flutter_tools/lib/src/base/fingerprint.dart 0% <0%> (-94.67%) ⬇️
...ages/flutter_tools/lib/src/macos/macos_device.dart 49.01% <0%> (-13.73%) ⬇️
...ckages/flutter_tools/lib/src/commands/upgrade.dart 24.44% <0%> (-10%) ⬇️
packages/flutter_tools/lib/src/base/utils.dart 87.62% <0%> (-5.95%) ⬇️
packages/flutter_tools/lib/src/ios/xcodeproj.dart 85.63% <0%> (-2.51%) ⬇️
packages/flutter_tools/lib/src/version.dart 90.99% <0%> (-1.9%) ⬇️
packages/flutter_tools/lib/src/base/os.dart 30.23% <0%> (-1.56%) ⬇️
packages/flutter_tools/lib/src/cache.dart 43.5% <0%> (-0.73%) ⬇️
... and 6 more

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 72888c7...85c61ca. Read the comment docs.

stuartmorgan-g
stuartmorgan-g approved these changes Sep 12, 2019
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with wording nit.

packages/flutter_tools/lib/src/base/user_messages.dart Outdated
'Visual Studio not installed; this is necessary for Windows development.\n'
'Download at https://visualstudio.microsoft.com/downloads/.';
'Download at https://visualstudio.microsoft.com/downloads/.\n'
'Please include the "$workload" workload, and the following components:\n ${components.join('\n ')}';
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Sep 12, 2019

Choose a reason for hiding this comment

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

I think this would be clearer given the installer structure as:
'Please install the "$workload" workload, including the following components:\'

(Let's change the "and include these components" in the message above to "including the following components" as well, for consistency between the messages.)

@franciscojma86 franciscojma86 merged commit 09bb07f into flutter:master Sep 13, 2019
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
@franciscojma86 @Inconnu08
Adds list required components when VS is not installed (flutter#40397)
0a0d60f
@franciscojma86 franciscojma86 deleted the install branch January 21, 2020 21:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

Assignees

@franciscojma86 franciscojma86

Labels

platform-windows Building on or for Windows specifically t: flutter doctor Problem related to the "flutter doctor" tool 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.

3 participants

@franciscojma86 @stuartmorgan-g @googlebot