[mustache_template] add example app and migrate README to excerpts#11333
[mustache_template] add example app and migrate README to excerpts#11333mozammal-hossain wants to merge 4 commits intoflutter:mainfrom
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the mustache_template package to version 2.0.4, introducing an example/ app to provide code snippets for the README.md via code-excerpt directives. The review notes that some nuanced lambda examples and documentation for LambdaContext.renderSource were removed from the README, and there's a syntax error in the docregion tags in example/main.dart that needs to be fixed.
Updated the example in main.dart to fix the doc region tags for nested paths, ensuring proper documentation generation. This change enhances clarity and maintains consistency in the example code.
|
Hi @ianloic, @precision, @sethladd, @romeo4934 — this PR is a docs/example update for mustache_template (restores nuanced lambda examples and LambdaContext.renderSource performance guidance), with no production/runtime behavior changes. Could someone please:
I also see tree-status is currently red; I’ll wait for tree reopen/normal merge guidance if needed. |
|
@mozammal-hossain Do not ping random people in PRs. You checked the box indicating that you read the contributing guidelines, which means you will have seen the multiple places where we say that reviews can take up to two weeks. Pinging people (especially the wrong people) almost immediately after opening the PR does not respect people's time, or our process. |
bkonyi
left a comment
There was a problem hiding this comment.
LGTM overall with one question.
|
|
||
| import 'package:mustache_template/mustache_template.dart'; | ||
|
|
||
| // #docregion BasicRender |
There was a problem hiding this comment.
What's with these #docregion comments?
There was a problem hiding this comment.
docregion = document region.
That means - same code has been written on the documentation(package's README.md).
stuartmorgan-g
left a comment
There was a problem hiding this comment.
No new package logic was introduced; this change adds documentation/example structure and excerpt wiring, so no additional tests were added
There is a lot of new code, in example/lib/main.dart. There's no reason it can be exercised in a test that ensures that the snippets are actually showing working (not just compiling) code.
See an earlier attempt at this issue for examples of what tests might look like.
| print(output); | ||
| } | ||
| ``` | ||
| Templates are parsed when created and can be rendered repeatedly with different |
There was a problem hiding this comment.
There's a lot of re-wrapping and re-wording of the README, which makes the diffs really hard to review. Please undo the README changes that aren't related to adopting snippets. If you believe the README needs rewriting, please file an issue describing why, and make a follow-up PR addressing it, so that we can evaluate those changes in isolation.
| ```dart | ||
| var t = Template('{{ author.name }}'); | ||
| var output = template.renderString({'author': {'name': 'Greg Lowe'}}); | ||
| void showStrictVsLenientBehavior() { |
There was a problem hiding this comment.
You've made all of the examples much longer without adding any substance, by including things like the print and the surrounding method definition. The excerpts should have the same scope they did before.
There was a problem hiding this comment.
I’ve trimmed the README excerpts back to the minimal snippets
stuartmorgan-g
left a comment
There was a problem hiding this comment.
(Fixing review state; that was meant to be a "Request changes".)
This PR adds a runnable
example/app formustache_templateand migrates README usage snippets to excerpt-managed code from that example. This addresses the missing example app noted in the issue and aligns the package with the repository’s code-excerpt documentation pattern.It also removes
mustache_templatefromscript/configs/temp_exclude_excerpt.yamlnow that excerpt support is in place, and includes the corresponding version/CHANGELOG updates.Fixes flutter/flutter#183936
Pre-Review Checklist
[shared_preferences]///).No new package logic was introduced; this change adds documentation/example structure and excerpt wiring, so no additional tests were added (test exemption).
If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2