Skip to content

Convert build mode to lowercase in tool_backend#40393

Merged
stuartmorgan-g merged 1 commit intoflutter:masterfrom
stuartmorgan-g:desktop-toolbackend-buildmode-lowercase
Sep 13, 2019
Merged

Convert build mode to lowercase in tool_backend#40393
stuartmorgan-g merged 1 commit intoflutter:masterfrom
stuartmorgan-g:desktop-toolbackend-buildmode-lowercase

Conversation

@stuartmorgan-g
Copy link
Contributor

Description

On macOS and iOS, the build mode is converted to lowercase in the
Flutter-side script, allowing projects to pass configuration names
directly. This adds the same behavior to the tool_backend.dart script
currently used by Windows and Linux, so that Windows builds can do the
same (since the default configuration names used by VS, as in Xcode,
start with a capital letter).

Related Issues

None

Tests

I added the following tests: None

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.

On macOS and iOS, the build mode is converted to lowercase in the
Flutter-side script, allowing projects to pass configuration names
directly. This adds the same behavior to the tool_backend.dart script
currently used by Windows and Linux, so that Windows builds can do the
same (since the default configuration names used by VS, as in Xcode,
start with a capital letter).
@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 12, 2019
@fluttergithubbot
Copy link
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.

@stuartmorgan-g
Copy link
Contributor Author

I know longer term Windows and Linux will migrate to assemble, but this is a trivial change that lets me simply the VS projects now.

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

LGTM

@stuartmorgan-g stuartmorgan-g added the a: desktop Running on desktop label Sep 12, 2019
@codecov
Copy link

codecov bot commented Sep 12, 2019

Codecov Report

Merging #40393 into master will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #40393     +/-   ##
=========================================
+ Coverage   58.56%   58.67%   +0.1%     
=========================================
  Files         192      192             
  Lines       18595    18627     +32     
=========================================
+ Hits        10890    10929     +39     
+ Misses       7705     7698      -7
Flag Coverage Δ
#flutter_tool 58.67% <ø> (+0.1%) ⬆️
Impacted Files Coverage Δ
packages/flutter_tools/lib/src/linux/makefile.dart 0% <0%> (-80%) ⬇️
...ackages/flutter_tools/lib/src/proxy_validator.dart 21.05% <0%> (-73.69%) ⬇️
...flutter_tools/lib/src/vscode/vscode_validator.dart 30% <0%> (-70%) ⬇️
packages/flutter_tools/lib/src/vscode/vscode.dart 13.95% <0%> (-37.21%) ⬇️
...lutter_tools/lib/src/fuchsia/fuchsia_workflow.dart 75% <0%> (-25%) ⬇️
packages/flutter_tools/lib/src/doctor.dart 59.72% <0%> (-13.34%) ⬇️
...es/flutter_tools/lib/src/commands/build_linux.dart 75% <0%> (-10%) ⬇️
...ackages/flutter_tools/lib/src/reporting/usage.dart 84.56% <0%> (-4.03%) ⬇️
...ages/flutter_tools/lib/src/base/user_messages.dart 45.45% <0%> (-3.64%) ⬇️
...s/flutter_tools/lib/src/windows/visual_studio.dart 79.41% <0%> (-2.41%) ⬇️
... and 18 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 576a455...8faa022. Read the comment docs.

@stuartmorgan-g stuartmorgan-g merged commit a7cfdbd into flutter:master Sep 13, 2019
@stuartmorgan-g stuartmorgan-g deleted the desktop-toolbackend-buildmode-lowercase branch September 13, 2019 02:13
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
On macOS and iOS, the build mode is converted to lowercase in the
Flutter-side script, allowing projects to pass configuration names
directly. This adds the same behavior to the tool_backend.dart script
currently used by Windows and Linux, so that Windows builds can do the
same (since the default configuration names used by VS, as in Xcode,
start with a capital letter).
@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.

Labels

a: desktop Running on desktop tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants