Hi @Hixie, would you please have a look at this PR, and/or suggest other reviewers?

This is the project that adds a more complete template (flutter create -t full, go/flutter-more-complete-template). Issue at #76379.

We have a 24-page public document explaining all the micro-decisions that went into this. I recommend skimming through it since it may pre-empt some understandable questions and raised eyebrows.

Sorry if this is in the linked docs, but what is the plan for testing this?

Relatedly, I'm wondering if this could be an opportunity for demonstrating testing practices by including a few more non-trivial tests in the template itself.

Sorry if this is in the linked docs, but what is the plan for testing this?

Is there a test for the current flutter create template (the "counter app") that we could look at for inspiration?

Relatedly, I'm wondering if this could be an opportunity for demonstrating testing practices by including a few more non-trivial tests in the template itself.

Here's our thinking on this. In short: we see that, with the current template, developers introduce breaking changes to the template in the first 5 minutes of their work, and then the smoke test is broken (often statically broken, so there are analyzer errors and squiggly lines). The effect is that the (otherwise very educational!) test/widget_test.dart file gets deleted or commented out. For that reason, the approach is to have relatively trivial tests that can't be broken (they don't depend on lib/) but that still provide some boilerplate (test('...', () {})) and links to documentation.

Nothing here is set in stone, though.

We need to find a way to not duplicate the existing platform files. We occasionally have to update these, and having 2 copies of everything is going to increase the chances that we accidentally get out of sync somewhere.

flutter create tests live in https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/commands.shard/permeable/create_test.dart. We currently generate the app, analyze it, check formatting, etc., but we don't have devicelab tests that run it, AFAIK.

We should definitely have tests for this app since it's non-trivial. I agree they shouldn't be in the template.
I think it might be worth branding this as a "best practices" template rather than "full" or "complete" or similar.
Thanks for the decision doc, that was very helpful. I think the analysis_options decision should be revisited in light of the comment I added to the doc. There's a large value in not having a file in the template there because everything in the template gets left behind when we update.
I concur with @jonahwilliams that we should make sure to not duplicate the platform-specific bits.

Thanks to everyone for the reviews! I'm working on addressing all feedback.

We should definitely have tests for this app since it's non-trivial.

Could you please describe in a bit more detail what kind of tests you'd like to see? I was going to replicate the tests for the Counter app, but I can include additional Widget and/or Unit tests to cover the functionality of the app as well.

Thanks again :)

Could you please describe in a bit more detail what kind of tests you'd like to see?

Well at a minimum we should verify that we can navigate to each page, and that changing the theme settings works. Might be good to have some golden tests too just in case.

Thanks again for the review, everyone. I've gone through and updated the tool a bit to support the new template. Features include:

1. Updated template based on feedback.
   • Remove analysis_options
   • Fix comments
   • Route restoration
   • Full types for all closures
   • use relative imports
2. Shared Platform Files. To share files, I've introduced the concept of a "merged" template, which combines several source templates & their template_images into a single destination.
   • templates/app_shared - Contains all files shared between different apps
   • templates/app - only files needed by counter app
   • templates/list_detail_app - only files needed by the new template.
   • Submitted PR to flutter_template_images to ensure the directory structure is in sync
3. Added tests
   • We've decided NOT to include any implementation tests in the final template, but we want to test the app before we ship it.
   • To support this Use Case, I've added an implentation-tests flag to the flutter create command. If this flag is set to true, the template will render .test.tmpl files found in the source directories and copy .golden.tmpl files from the flutter_template_images repo.

Please note: some tests will not pass until we merge flutter/packages#295.

This is my first time modifying the CLI, so I'm not sure if this is the correct approach! Happy to change it up if need be.

Thanks everyone. I agree: There's no perfect name here, and I'm happy with skeleton as well :)

I saw some updates were pushed to this branch, but they looked a bit busted. There should only be about ~900 additions, nowhere near 8000. Therefore, I've rebased my branch against master to be sure this PR is up-to-date, renamed everything to skeleton, and force-pushed once again.

At this point, I think the plan for landing this PR seems to be:

1. Merge flutter_template_images: 3.0.0, which renames list_detail_app to skeleton: [flutter_template_images] Rename list_detail_app to skeleton packages#310
2. Get any final input from the team and update this PR (all outstanding feedback so far has been addressed)
3. Rerun the CI checks for this PR and fix any issues. The flutter_tools pubspec for this PR already references flutter_template_images: 3.0.0, but of course no tests will pass until that's deployed.
4. Merge it up!

cc @stuartmorgan re: the use of non-default templates using a naming convention like "skeleton_app" or "federated_plugin" as he's particularly interested in adding a federated plugin template in the future.

I think it's plausible that we'll need a couple of new *_plugin types, but I don't have a strong opinion on whether that means this needs an *_app suffix; I don't see an issue with adding *_plugin templates even if this doesn't end with _app

The reason I'm not 100% on board with *_app is that it's not great developer experience on the command line. flutter create -t skeleton is easy to write and remember. flutter create -t skeleton_app not so much. ("Was it skeleton_app? Or skeleton_application? Or skeletonApp?")

I do understand the appeal of having a more stratified approach to naming. I just think we need to understand the tradeoff between that, and developer experience.

Perhaps we could have a policy that non-app templates get the suffix, e.g. skeleton is an app template but instead of using federated, we use federated_plugin.

We could also drop the underscore, as in "skeletonapp"

#78619 is migrating the other templates to null safety. This one probably also needs to be null save then.

/cc @mit-mit

Hey all -- not 100% sure where we stand with this one. Unfortunately, I do not have any more free time to dedicate to this PR. Therefore, I'll close this one out for now.

Please feel free to pick it up if you'd like to continue with it!

@brianegan what was left to do? wasn't it reviewed and ready to land?

It was ready to roll with the old name or "skeleton," but from what I can gather the naming discussion has not come to a conclusion and the PR has become a bit stale.

Therefore, to wrap up this work, the team needs to choose a name, update the template to support the new name, update the template to support null safety, and rebase the branch against master again as there are now conflicting changes.

If someone is going to pick this up and drive it to completion please let me know soon, because otherwise flutter/packages#295 needs to be reverted as a second breaking change to that package.

In general, this template has a ton of great practices in a very little bit of code. I'm looking forward to using it in my own projects.

Here's a little bit of review feedback for @filiph as referenced to https://github.com/brianegan/new_flutter_template:

• the settings page action is hidden behind the debug banner, which could be mitigated with a tooltip
• I would expect settings to be a dialog on the desktop form factor and not a page to navigate to
• I normally find myself having a shared Scaffold and AppBar between pages to reduce the duplicated code while providing a consistent UI between pages
• the item details page has no Settings actions
• I would expect the strings "Dummy Items" and "Item Details" to be in the arb and for the code to use them
• all of the tests except the first two in implementation_test.dart have great comments; would like great comments in all tests

Thanks @csells, replies inline:

the settings page action is hidden behind the debug banner, which could be mitigated with a tooltip

We were aware of the issue but decided to prioritize simplicity of code. What do you mean by mitigating the issue with a tooltip?

I would expect settings to be a dialog on the desktop form factor and not a page to navigate to

That would make the code a lot more complex, and isn't necessarily all that clear cut. Desktop apps often open a completely separate window for settings screens, which would make things even harder to implement.

I normally find myself having a shared Scaffold and AppBar between pages to reduce the duplicated code while providing a consistent UI between pages

We explored this. In many cases, AppBar will be meaningfully different between pages. This can be mitigated by creating a custom widget, of course, but that would make the template more complex. And it's something where we don't need to educate. The template educates about things like file structure, but extracting widget is something an intermediate developer is perfectly capable of.

In short, this goes back to the goals of the template: getting out of the way of an intermediate/advanced developer.

the item details page has no Settings actions

That is intentional. In most apps we're seeing, app-level settings aren't accessible from everywhere. Of course, a desktop app has its settings in the menu, so in that sense, it's available everywhere. But again, this is something an intermediate developer should be able to implement themselves. And it allows us to save code that would otherwise need to be understood, modified and/or deleted by the developer.

I would expect the strings "Dummy Items" and "Item Details" to be in the arb and for the code to use them
all of the tests except the first two in implementation_test.dart have great comments; would like great comments in all tests

This is also intentional. We've seen that developers, when starting projects, don't immediately implement i18n everywhere. It really damages perceived developer productivity when, instead of Text('OK') you have to write the i18n equivalent (in several files).

In the template, we show how to go about doing i18n in Flutter, and give you the files in the right places and with the right contents, but don't implement it fully for you. As a developer, you'd be deleting most of it anyway.

ping @csells

@filiph replies:

re: mitigating with a tooltip: at least a tooltip would show if you move the mouse over in that direction, although obviously that's a pretty lame mitigation.

I'm curious what feedback we get from a wider audience on the rest. Certainly what you have is a big improvement in pointing people down the right path structurally compared to the Counter app.

Is there an updated PR to track work on this?

Not yet. I'm planning to create it after next week (I/O).

Not yet. I'm planning to create it after next week (I/O).

FYI @filiph the flutter/packages change got reverted flutter/packages#353. You'll need to re-land that in order to land the tool change.

You'll need to re-land that in order to land the tool change.

Given the experience last time, where the images package was left in an unusable state for several months, I would strongly prefer that the updated PR be fully approved and ready to land before doing the changes it the images package. It can be tested locally with version 2.0.0 of the package.

Hey all, I'm unable to push changes to this PR for whatever reason, so I'm recreating it in #83530. That PR is ready for review. I'll ask folks over there.

This PR can stay closed.