Skip to content

Conversation

@stuartmorgan-g
Copy link
Contributor

Description

Moves files generated in windows/flutter/ as part of the build to an ephemeral/ subdirectory, matching the approach used on macOS (and in the future, Windows).

Adds that directory to the generated properties file to minimize hard-coding of paths in the project.

Tests

I added the following tests: None. Updated existing tests.

Replace this with a list of the tests that you added as part of this PR. A change in behaviour with no test covering it
will likely get reverted accidentally sooner or later. PRs must include tests for all changed/updated/fixed behaviors. See Test Coverage.

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.

This is a breaking change for Windows, which is explicitly not stable.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 10, 2019
@stuartmorgan-g stuartmorgan-g added the a: desktop Running on desktop label Sep 10, 2019
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.

LGTM

@codecov
Copy link

codecov bot commented Sep 11, 2019

Codecov Report

Merging #40194 into master will increase coverage by 0.6%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #40194     +/-   ##
=========================================
+ Coverage   58.11%   58.72%   +0.6%     
=========================================
  Files         192      192             
  Lines       18443    18444      +1     
=========================================
+ Hits        10719    10831    +112     
+ Misses       7724     7613    -111
Flag Coverage Δ
#flutter_tool 58.72% <80%> (+0.6%) ⬆️
Impacted Files Coverage Δ
...s/flutter_tools/lib/src/windows/build_windows.dart 85.36% <100%> (+0.36%) ⬆️
packages/flutter_tools/lib/src/project.dart 83.8% <75%> (+0.05%) ⬆️
packages/flutter_tools/lib/src/commands/drive.dart 5.83% <0%> (-69.17%) ⬇️
packages/flutter_tools/lib/src/web/web_device.dart 41.17% <0%> (-9.81%) ⬇️
...ges/flutter_tools/lib/src/application_package.dart 64.41% <0%> (-2.25%) ⬇️
packages/flutter_tools/lib/src/version.dart 90.9% <0%> (-1.92%) ⬇️
...utter_tools/lib/src/macos/cocoapods_validator.dart 81.81% <0%> (-0.8%) ⬇️
packages/flutter_tools/lib/src/cache.dart 43.5% <0%> (-0.73%) ⬇️
packages/flutter_tools/lib/src/base/process.dart 87.09% <0%> (+0.64%) ⬆️
.../flutter_tools/lib/src/runner/flutter_command.dart 78.5% <0%> (+0.87%) ⬆️
... and 9 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 f7d86a5...cf03aa7. Read the comment docs.

@stuartmorgan-g stuartmorgan-g merged commit e6ae95c into flutter:master Sep 11, 2019
@stuartmorgan-g stuartmorgan-g deleted the windows-ephemeral branch September 11, 2019 14:47
stuartmorgan-g added a commit to google/flutter-desktop-embedding that referenced this pull request Sep 11, 2019
Updates the Windows builds for the breaking change in
flutter/flutter#40194

Note: plugins continue to hard-code the cache dir instead of using
FLUTTER_EPHEMERAL_DIR since currently they share the generated
properties file, but need a project-local copy of the cache. That
whole structure needs to be reworked, so the fact that it's
currently hard-coded isn't worth trying to fix right now
(e.g., by making FLUTTER_EPHEMERAL_DIR relative).
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
Moves files generated in windows/flutter/ as part of the build to an ephemeral/ subdirectory, matching the approach used on macOS (and in the future, Windows).

Adds that directory to the generated properties file to minimize hard-coding of paths in the project.
@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