Skip to content

Adjust phrasing of features #36874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jonahwilliams merged 3 commits into from
Jul 25, 2019
Merged

Adjust phrasing of features #36874

jonahwilliams merged 3 commits into from
Jul 25, 2019

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jul 24, 2019

Description

Separate feature description and where it is enabled into two sentences, following suggestion by @devoncarew . Adjust names of features to better match brand guidelines.

Usage: flutter config [arguments]
-h, --help                               Print this usage information.
    --[no-]analytics                     Enable or disable reporting anonymously tool usage statistics and crash reports.
    --clear-ios-signing-cert             Clear the saved development certificate choice used to sign apps for iOS device
                                         deployment.

    --gradle-dir                         The gradle install directory.
    --android-sdk                        The Android SDK directory.
    --android-studio-dir                 The Android Studio install directory.
    --[no-]enable-web                    Enable or disable Flutter for Web. This setting will take effect on master, dev
                                         channels.

    --[no-]enable-linux-desktop          Enable or disable Flutter for Linux Desktop. This setting will take effect on the
                                         master channel.

    --[no-]enable-macos-desktop          Enable or disable Flutter for macOS Desktop. This setting will take effect on the
                                         master channel.

    --[no-]enable-windows-desktop        Enable or disable Flutter for Windows Desktop. This setting will take effect on the
                                         master channel.

    --[no-]enable-build-plugin-as-aar    Enable or disable Build plugins independently as AARs in app projects. This setting
                                         will take effect on master, dev channels.

    --clear-features                     Remove all configured features and restore them to the default values.

@jonahwilliams jonahwilliams added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 24, 2019
christopherfujino
christopherfujino approved these changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

lgtm

devoncarew
devoncarew reviewed Jul 24, 2019
View reviewed changes
packages/flutter_tools/test/general.shard/features_test.dart Outdated

test('flutter web help string', () {
expect(flutterWebFeature.generateHelpMessage(), 'Enable or disable Flutter Web on master, dev channels.');
expect(flutterWebFeature.generateHelpMessage(), 'Enable or disable Flutter for Web. This setting will take effect on master, dev channels.');
Copy link
Contributor

@devoncarew devoncarew Jul 24, 2019

Choose a reason for hiding this comment

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

"Flutter for Web" or "Flutter for web"?

And, perhaps `"This setting will take effect on the master and dev channels.".

devoncarew
devoncarew approved these changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

lgtm w/ some potential additional word-smithing

devoncarew
devoncarew reviewed Jul 24, 2019
View reviewed changes
packages/flutter_tools/lib/src/features.dart Outdated
/// The [Feature] for flutter web.
const Feature flutterWebFeature = Feature(
name: 'Flutter Web',
name: 'Flutter for Web',
Copy link
Contributor

@devoncarew devoncarew Jul 24, 2019

Choose a reason for hiding this comment

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

Also here, "Flutter for Web" ==> "Flutter for web"

@jonahwilliams
Copy link
Contributor Author

jonahwilliams commented Jul 24, 2019

Updated usage:

Usage: flutter config [arguments]
-h, --help                               Print this usage information.
    --[no-]analytics                     Enable or disable reporting anonymously tool usage statistics and crash reports.
    --clear-ios-signing-cert             Clear the saved development certificate choice used to sign apps for iOS device
                                         deployment.

    --gradle-dir                         The gradle install directory.
    --android-sdk                        The Android SDK directory.
    --android-studio-dir                 The Android Studio install directory.
    --[no-]enable-web                    Enable or disable Flutter for web. This setting will take effect on the master and dev
                                         channels.

    --[no-]enable-linux-desktop          Enable or disable Flutter for Linux desktop. This setting will take effect on the
                                         master channel.

    --[no-]enable-macos-desktop          Enable or disable Flutter for macOS desktop. This setting will take effect on the
                                         master channel.

    --[no-]enable-windows-desktop        Enable or disable Flutter for Windows desktop. This setting will take effect on the
                                         master channel.

    --[no-]enable-build-plugin-as-aar    Enable or disable Build plugins independently as AARs in app projects. This setting
                                         will take effect on the master and dev channels.

    --clear-features                     Remove all configured features and restore them to the default values.

Run "flutter help" to see global options.

devoncarew
devoncarew reviewed Jul 24, 2019
View reviewed changes
packages/flutter_tools/lib/src/features.dart Outdated
/// The [Feature] for macOS desktop.
const Feature flutterMacOSDesktopFeature = Feature(
name: 'Flutter Desktop for macOS',
name: 'Flutter for macOS desktop',
Copy link
Contributor

@devoncarew devoncarew Jul 24, 2019

Choose a reason for hiding this comment

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

"Flutter for macOS desktop" ==> "Flutter for desktop on macOS"? "Flutter for desktop (macOS)"?

The changes above would let the name read Flutter for desktop (which would parallel the Flutter for web name).

lgtm either way

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jul 24, 2019

Choose a reason for hiding this comment

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

Done

@codecov
Copy link

codecov bot commented Jul 25, 2019 •
edited
Loading

Codecov Report

Merging #36874 into master will increase coverage by 0.19%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #36874      +/-   ##
==========================================
+ Coverage   55.31%   55.51%   +0.19%     
==========================================
  Files         190      190              
  Lines       17603    17594       -9     
==========================================
+ Hits         9737     9767      +30     
+ Misses       7866     7827      -39
Flag Coverage Δ
#flutter_tool 55.51% <100%> (+0.19%) ⬆️
Impacted Files Coverage Δ
packages/flutter_tools/lib/src/features.dart 100% <100%> (ø) ⬆️
...ackages/flutter_tools/lib/src/resident_runner.dart 35.94% <0%> (-17.27%) ⬇️
...ackages/flutter_tools/lib/src/commands/config.dart 79.16% <0%> (-6.74%) ⬇️
packages/flutter_tools/lib/src/cache.dart 47.05% <0%> (-0.77%) ⬇️
packages/flutter_tools/lib/src/vmservice.dart 39.13% <0%> (-0.64%) ⬇️
...flutter_tools/lib/src/test/coverage_collector.dart 60.95% <0%> (-0.47%) ⬇️
.../flutter_tools/lib/src/android/android_device.dart 33.5% <0%> (-0.26%) ⬇️
.../flutter_tools/lib/src/runner/flutter_command.dart 82.35% <0%> (-0.08%) ⬇️
packages/flutter_tools/lib/src/commands/run.dart 60.76% <0%> (ø) ⬆️
packages/flutter_tools/lib/src/dart/pub.dart 79.34% <0%> (ø) ⬆️
... and 19 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 15e322c...fe5602c. Read the comment docs.

@jonahwilliams jonahwilliams merged commit 76d0581 into flutter:master Jul 25, 2019
@jonahwilliams jonahwilliams deleted the adjust_feature_phrasing branch July 25, 2019 15:48
johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019
@johnsonmh
Adjust phrasing of features (flutter#36874)
972ca11 
* adjust phrasing of features

* word smithing

* more wordsmithing
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@devoncarew devoncarew devoncarew approved these changes

@christopherfujino christopherfujino christopherfujino 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.

4 participants

@jonahwilliams @devoncarew @christopherfujino @googlebot