Skip to content

Conversation

@thkim1011
Copy link
Contributor

Currently, nullable-getter defaults to false when l10n.yaml is not present, which is not the same behavior as when an l10n.yaml file is present and nullable-getter is not set.

Fixes #120457.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 6, 2023
@flutter flutter deleted a comment from flutter-dashboard bot Apr 6, 2023
..createSync(recursive: true);
arbFile.writeAsStringSync('''
{
"helloWorld": "Hello, World!",
Copy link
Contributor

Choose a reason for hiding this comment

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

is this testing nullable getters?

Copy link
Contributor Author

@thkim1011 thkim1011 Apr 6, 2023

Choose a reason for hiding this comment

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

I'm not sure if I understand your question.

The helloWorld string is basic string to get the localizations file to generate.

Then we test that the generated file has

// With nullable-getter true
static AppLocalizations? of(BuildContext context)

instead of

// With nullable-getter false
static AppLocalizations of(BuildContext context)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh ok, thanks!

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

ProcessManager: () => FakeProcessManager.any(),
});

testUsingContext('nullable-getter defaults to true', () async {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is failing on Windows:

00:38 +368: test/commands.shard\hermetic\generate_localizations_test.dart: nullable-getter defaults to true
 Exception: The 'arb-dir' directory, 'MemoryDirectory: 'lib\l10n'', does not exist.
 Make sure that the correct path was provided.
 #0      throwToolExit (package:flutter_tools/src/base/common.dart:10:3)
 #1      GenerateLocalizationsCommand.runCommand (package:flutter_tools/src/commands/generate_localizations.dart:300:9)

Maybe the MemoryFileSystem, when constructed on windows (on line 29), needs to be passed the path separator type.

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

@thkim1011 thkim1011 added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 18, 2023
@auto-submit auto-submit bot merged commit b6c7df4 into flutter:master Apr 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 19, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Apr 19, 2023
flutter/flutter@42fb0b2...3476b96

2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810)
2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097)
2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094)
2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078)
2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253)
2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336)
2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069)
2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879)
2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102)
2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026)
2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000)
2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060)
2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899)
2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056)
2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981)
2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057)
2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805)
2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353)
2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053)
2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
flutter/flutter@42fb0b2...3476b96

2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810)
2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097)
2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094)
2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078)
2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253)
2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336)
2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069)
2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879)
2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102)
2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026)
2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000)
2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060)
2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899)
2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056)
2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981)
2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057)
2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805)
2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353)
2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053)
2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Existing l10n.yaml file is mandatory or null check assertion is thrown in AppLocalizations.of

2 participants