Adaptions for Flutter Gallery demo #736
Conversation
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthPackage publish validation ✔️Details
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. Changelog Entry ✔️Details
Changes to files need to be accounted for in their respective changelogs. Coverage ✔️Details
This check for test coverage is informational (issues shown here will not fail the PR). Breaking changes ✔️Details
|
|
nit and not necessarily related to this RP - we're now using |
| }); | ||
|
|
||
| static Future<GenerationOptions> fromPubspec(BuildStep buildStep) async { | ||
| final pubspecId = await buildStep.findAssets(Glob('pubspec.yaml')).first; | ||
| final pubspecData = await buildStep.readAsString(pubspecId); | ||
| final pubspec = loadYaml(pubspecData) as YamlMap; | ||
| final messagesOptions = pubspec['messages'] as YamlMap; | ||
| final messagesOptions = pubspec['messages'] as YamlMap?; |
There was a problem hiding this comment.
Looking at the yaml files this is read from, the messages: key lacks a bit of context.
I do like that fact that we're not creating yet another data file in the user's directory (we have some to configure build, test, dartdoc, ...).
Should this key be messages_builder:? Should it be in a new 'options' section?
name: my_package
...
dependencies:
path: any
...
options:
messages_builder:
use_method_names: true
There was a problem hiding this comment.
I slightly prefer putting these new package options into a new category (options:), but realize there's a high degree of bike-shedding here.
There was a problem hiding this comment.
I have no opinion on whether it should be in a new options: key, but it feels a bit redundant. What is the pubspec if not a collection of options? I took the inspiration from Flutter, where top-level keys are common I believe.
There was a problem hiding this comment.
Flutter's are in a flutter: section; it feels large enough to get it's own section.
Mostly the top-level keys are for use by pub; it's less common for tools or packages to introduce new keys.
I think the important thing would be for somebody reading the file:
messages:
generateMethods: true
generateFindById: false
generateFindBy: integer
to know that:
- these keys aren't mistaken for pub-related things
- are pretty clear what they're associated with (the package that reads them? the functionality used by the package under development - localization, message generation, ... ?)
messages: seems a little bare to me - slightly too little context. You might want play around with a few other keys / nested keys to see if something jumps out at you.
(I did see after writing this comment that you'd changed this to messages_builder:)
| }); | ||
|
|
||
| static Future<GenerationOptions> fromPubspec(BuildStep buildStep) async { | ||
| final pubspecId = await buildStep.findAssets(Glob('pubspec.yaml')).first; | ||
| final pubspecData = await buildStep.readAsString(pubspecId); | ||
| final pubspec = loadYaml(pubspecData) as YamlMap; | ||
| final messagesOptions = pubspec['messages'] as YamlMap; | ||
| final messagesOptions = pubspec['messages'] as YamlMap?; |
There was a problem hiding this comment.
Flutter's are in a flutter: section; it feels large enough to get it's own section.
Mostly the top-level keys are for use by pub; it's less common for tools or packages to introduce new keys.
I think the important thing would be for somebody reading the file:
messages:
generateMethods: true
generateFindById: false
generateFindBy: integer
to know that:
- these keys aren't mistaken for pub-related things
- are pretty clear what they're associated with (the package that reads them? the functionality used by the package under development - localization, message generation, ... ?)
messages: seems a little bare to me - slightly too little context. You might want play around with a few other keys / nested keys to see if something jumps out at you.
(I did see after writing this comment that you'd changed this to messages_builder:)
A series of changes for flutter/gallery#1021.
arbfiles with different ordering and message keys than their reference filearbfile is the reference fileAfter this PR, the plan is to publish experimental(!) versions of the packages to ease testing with other packages and applications.
Contribution guidelines:
dart format.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.