-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[hooks] Document flutter create --template=package_ffi
#12845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @dcharkes, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly updates the Flutter documentation regarding Foreign Function Interface (FFI) usage. It establishes Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request does a great job of updating the FFI documentation to recommend the new package_ffi template. The new documentation page is clear and comprehensive, and the legacy pages are correctly marked with warnings explaining the valid use cases for the old approach. I've added a couple of minor suggestions to improve consistency in the documentation.
|
Visit the preview URL for this PR (updated for commit 202d0ea): https://flutter-docs-prod--pr12845-build-hooks-docs-o35es4kr.web.app |
sfshaza2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits about a list. Otherwise lgtm
That's not an FFI plugin though, that's an actual plugin that happens to use FFI as a communication mechanism. It shouldn't require any special template that I'm aware of.
Yes, we should definitely update the docs as part of deprecating the template. (To clarify, by "as part of" I just mean as part of the overall process; I don't see any reason we can't switch docs to recommending hooks immediately, even though template deprecation will take a while to reach |
antfitch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small change to make sure we don't have all of these c-interop.md files in then TOC anymore.
Right, we should point to the normal plugin documentation then. However, if users end up adding C/C++ code to their plugins to use via |
Co-authored-by: Shams Zakhour <44418985+sfshaza2@users.noreply.github.com>
|
@antfitch The preview website doesn't want to load https://flutter-docs-prod--pr12845-build-hooks-docs-o35es4kr.web.app/platform-integration/c-interop ? It forwards to https://flutter-docs-prod--pr12845-build-hooks-docs-o35es4kr.web.app/platform-integration/ . |
Updated the link for recommended C interop approach.
Updated the link for C interop build hooks in iOS documentation.
Updated the link for C interop build hooks in the documentation.
Updated the documentation to clarify the use of the `flutter create --template=package_ffi` command and the role of build hooks in native code integration.
antfitch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating the TOC. I've also updated the filename for the new topic to be 'bind-native-code.md' since it seems more accurate and should not present issues with redirects.
The Flutter docs were still recommending
plugin_ffifor interop instead ofpackage_ffi.This leads to LLMs generating Plugin FFI code instead of build hooks. @antfitch believes making our documentation up to date should lead to LLMs generating the right thing.
The situation is not as clear-cut as simply deleting the old docs, as there are still valid use cases for using FFI plugins such as accessing the Flutter plugin API. However, it should be clear that the new recommended way for bundling dynamic libraries or accessing system libraries via build hooks and
@Native() externalfunctions.cc @stuartmorgan-g How do you feel about marking the old FFI plugins as deprecated in the docs in accordance with flutter/flutter#131209? Flutter Plugins (non-FFI) are not deprecated. It's the use cases using
dart:ffiand FFIgen that should use the new path mostly.FYI @jwill You might also be looking into docs.
cc @mariamhas This is intended to address the EAP feedback.