fix: wallet app example build with flutter 3.27.1 #435
Conversation
|
To view this pull requests documentation preview, visit the following URL: docs.page/focustree/starknet.dart~435 Documentation is deployed and generated using docs.page. |
WalkthroughThis pull request introduces comprehensive changes to the mobile wallet example application, focusing on the Linux platform configuration. The modifications include updates to Gradle build files, CMake configurations, and Flutter-specific Linux integration files. The changes aim to improve the project structure, update build tools, and provide a more robust Linux desktop application setup. Changes
Sequence DiagramsequenceDiagram
participant User
participant HomeScreen
participant Scaffold
participant Layout2
participant DeployAccountButton
User->>HomeScreen: Interact with screen
HomeScreen->>Scaffold: Wrap Layout2
Scaffold->>Layout2: Render body
Layout2->>DeployAccountButton: Add to Column
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
examples/wallet_app/linux/runner/my_application.cc (4)
103-107: Avoid Redundant Null AssignmentIn
my_application_dispose, after callingg_clear_pointer,self->dart_entrypoint_argumentsis already set toNULL. AssigningNULLagain is redundant.No action needed as
g_clear_pointerhandles nullification. You can remove any additional assignments toself->dart_entrypoint_arguments.
30-39: Simplify Header Bar Logic for Improved ReadabilityThe conditional logic determining the use of a header bar can be streamlined for clarity.
Consider refactoring as follows:
gboolean use_header_bar = TRUE; #ifdef GDK_WINDOWING_X11 - GdkScreen* screen = gtk_window_get_screen(window); - if (GDK_IS_X11_SCREEN(screen)) { - const gchar* wm_name = gdk_x11_screen_get_window_manager_name(screen); - if (g_strcmp0(wm_name, "GNOME Shell") != 0) { - use_header_bar = FALSE; - } - } + GdkDisplay* display = gdk_display_get_default(); + if (GDK_IS_X11_DISPLAY(display)) { + const gchar* wm_name = gdk_x11_display_get_window_manager_name(display); + if (g_strcmp0(wm_name, "GNOME Shell") != 0) { + use_header_bar = FALSE; + } + } #endif
86-91: Remove Unused Variable and CommentsIn
my_application_startup, the variableselfis declared but not used. Additionally, placeholder comments can be removed to clean up the code.Apply this diff to remove the unused variable and comments:
-static void my_application_startup(GApplication* application) { - // MyApplication* self = MY_APPLICATION(application); - - // Perform any actions required at application startup. +static void my_application_startup(GApplication* application) { G_APPLICATION_CLASS(my_application_parent_class)->startup(application); }
94-100: Clean Upmy_application_shutdownFunctionSimilarly, in
my_application_shutdown, the unused variableselfand placeholder comments can be removed.Apply this diff:
-static void my_application_shutdown(GApplication* application) { - // MyApplication* self = MY_APPLICATION(application); - - // Perform any actions required at application shutdown. +static void my_application_shutdown(GApplication* application) { G_APPLICATION_CLASS(my_application_parent_class)->shutdown(application); }examples/wallet_app/linux/runner/main.cc (1)
3-6: Simplify Memory Management withg_autoptrUsing
g_autoptrforMyApplicationis appropriate here as it ensures automatic cleanup. However, sinceg_application_runmanages the lifecycle, usingg_autoptrmight be unnecessary.Consider simplifying the code:
int main(int argc, char** argv) { - g_autoptr(MyApplication) app = my_application_new(); + MyApplication* app = my_application_new(); return g_application_run(G_APPLICATION(app), argc, argv); }Ensure that not using
g_autoptrdoes not introduce memory leaks.🧰 Tools
🪛 cppcheck (2.10-2)
[error] 6-6: There is an unknown macro here somewhere. Configuration is required. If G_DECLARE_FINAL_TYPE is a macro then please configure it.
(unknownMacro)
examples/wallet_app/linux/runner/CMakeLists.txt (1)
1-26: LGTM! Well-structured CMake configuration.The configuration properly sets up the Linux runner with all necessary dependencies and build settings.
Consider adding brief comments explaining the purpose of each library in the
target_link_librariessection for better maintainability.# Add dependency libraries. Add any application-specific dependencies here. target_link_libraries(${BINARY_NAME} PRIVATE flutter) -target_link_libraries(${BINARY_NAME} PRIVATE PkgConfig::GTK) +# GTK: Required for Linux desktop window management and native UI integration +target_link_libraries(${BINARY_NAME} PRIVATE PkgConfig::GTK)examples/wallet_app/linux/flutter/CMakeLists.txt (1)
37-58: Consider using a more maintainable approach for header listThe long list of Flutter library headers could be moved to a separate file for better maintainability.
Consider creating a separate
flutter_headers.cmakefile and including it:-list(APPEND FLUTTER_LIBRARY_HEADERS - "fl_basic_message_channel.h" - ... - "flutter_linux.h" -) +include(flutter_headers.cmake)examples/wallet_app/linux/CMakeLists.txt (2)
7-10: Review application identifierThe application ID uses a generic example domain. Consider updating it to match your organization's domain structure.
-set(APPLICATION_ID "com.example.wallet_app") +set(APPLICATION_ID "com.coderabbit.wallet_app")
42-47: Consider enhancing compiler settingsThe standard settings function could be improved:
- C++14 is relatively old
- Missing important compiler flags
Consider updating the compiler settings:
function(APPLY_STANDARD_SETTINGS TARGET) - target_compile_features(${TARGET} PUBLIC cxx_std_14) - target_compile_options(${TARGET} PRIVATE -Wall -Werror) + target_compile_features(${TARGET} PUBLIC cxx_std_17) + target_compile_options(${TARGET} PRIVATE + -Wall + -Werror + -Wextra + -Wpedantic + -Wno-unused-parameter + ) target_compile_options(${TARGET} PRIVATE "$<$<NOT:$<CONFIG:Debug>>:-O3>") target_compile_definitions(${TARGET} PRIVATE "$<$<NOT:$<CONFIG:Debug>>:NDEBUG>") endfunction()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
docs/examples/mobile-wallet.mdx(1 hunks)examples/wallet_app/android/app/build.gradle(1 hunks)examples/wallet_app/android/build.gradle(0 hunks)examples/wallet_app/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)examples/wallet_app/android/settings.gradle(1 hunks)examples/wallet_app/lib/screens/home_screen.dart(1 hunks)examples/wallet_app/linux/.gitignore(1 hunks)examples/wallet_app/linux/CMakeLists.txt(1 hunks)examples/wallet_app/linux/flutter/CMakeLists.txt(1 hunks)examples/wallet_app/linux/flutter/generated_plugin_registrant.cc(1 hunks)examples/wallet_app/linux/flutter/generated_plugin_registrant.h(1 hunks)examples/wallet_app/linux/flutter/generated_plugins.cmake(1 hunks)examples/wallet_app/linux/runner/CMakeLists.txt(1 hunks)examples/wallet_app/linux/runner/main.cc(1 hunks)examples/wallet_app/linux/runner/my_application.cc(1 hunks)examples/wallet_app/linux/runner/my_application.h(1 hunks)
💤 Files with no reviewable changes (1)
- examples/wallet_app/android/build.gradle
✅ Files skipped from review due to trivial changes (6)
- examples/wallet_app/linux/flutter/generated_plugin_registrant.cc
- examples/wallet_app/linux/flutter/generated_plugin_registrant.h
- examples/wallet_app/linux/.gitignore
- examples/wallet_app/android/gradle/wrapper/gradle-wrapper.properties
- docs/examples/mobile-wallet.mdx
- examples/wallet_app/linux/flutter/generated_plugins.cmake
🧰 Additional context used
🪛 cppcheck (2.10-2)
examples/wallet_app/linux/runner/main.cc
[error] 6-6: There is an unknown macro here somewhere. Configuration is required. If G_DECLARE_FINAL_TYPE is a macro then please configure it.
(unknownMacro)
examples/wallet_app/linux/runner/my_application.cc
[error] 6-6: There is an unknown macro here somewhere. Configuration is required. If G_DECLARE_FINAL_TYPE is a macro then please configure it.
(unknownMacro)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: analyze
- GitHub Check: test-integration
🔇 Additional comments (10)
examples/wallet_app/linux/runner/main.cc (1)
1-1: Ensure Build Configuration Includes Header FilesThe inclusion of
"my_application.h"requires that the build system correctly locates this header file. Verify that the build configuration includes the appropriate include paths.If you encounter compilation issues, adjust the include directories in your build system to reference the
runnerdirectory.examples/wallet_app/linux/runner/my_application.h (1)
4-4:⚠️ Potential issueInclude
<glib-object.h>for Macro DefinitionsThe macro
G_DECLARE_FINAL_TYPErequires<glib-object.h>to be included. Currently, only<gtk/gtk.h>is included, which may not define this macro.Apply this diff to include the necessary header:
#include <gtk/gtk.h> +#include <glib-object.h> G_DECLARE_FINAL_TYPE(MyApplication, my_application, MY, APPLICATION, GtkApplication)This ensures that
G_DECLARE_FINAL_TYPEis properly defined, resolving any issues with unknown macros.Likely invalid or redundant comment.
examples/wallet_app/lib/screens/home_screen.dart (1)
10-27: LGTM! Good improvements to the widget structure.The changes improve the widget hierarchy by:
- Adding a proper Material Design Scaffold
- Better organizing the layout with proper alignment properties
- Including the DeployAccountButton for account deployment functionality
examples/wallet_app/android/settings.gradle (1)
1-23: LGTM! Good modernization of Gradle configuration.The changes follow modern Gradle best practices with proper plugin management and explicit version declarations.
Let's verify the plugin versions are the latest stable ones compatible with Flutter 3.27.1:
examples/wallet_app/android/app/build.gradle (2)
1-5: LGTM! Good adoption of modern Gradle plugins DSL.The change to use the plugins block is a good modernization of the build configuration.
Line range hint
45-46: Address TODO comments in the configuration.There are two TODO items that should be addressed:
- Application ID should be specified properly for production
- Release signing configuration needs to be set up
Would you like me to help create a GitHub issue to track these TODOs?
Also applies to: 56-59
examples/wallet_app/linux/flutter/CMakeLists.txt (3)
24-27: LGTM: System dependencies are properly configuredThe required system packages (GTK, GLIB, GIO) are correctly specified using pkg_check_modules.
2-2: Verify CMake version compatibility with Flutter 3.27.1The minimum CMake version is set to 3.10, which is quite old. While this works, newer Flutter versions might benefit from newer CMake features.
✅ Verification successful
CMake version 3.10 requirement is correct for Linux Flutter builds
The CMake version requirement of 3.10 is consistent across all Linux Flutter builds in the codebase, while Windows builds use 3.14. This platform-specific difference is intentional and follows Flutter's build system requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check CMake version requirements in Flutter documentation gh api repos/flutter/flutter/contents/packages/flutter_tools/templates/app/linux.tmpl/CMakeLists.txt | jq -r '.content' | base64 -d | grep -A 2 'cmake_minimum_required'Length of output: 288
Script:
#!/bin/bash # Find all CMakeLists.txt files and their cmake_minimum_required versions fd CMakeLists.txt --type f --exec grep -l "cmake_minimum_required" {} \; | while read -r file; do echo "=== $file ===" grep -A 1 "cmake_minimum_required" "$file" done # Look for Flutter version specification fd . --type f -e yaml -e md | xargs grep -l "flutter" | while read -r file; do echo "=== $file ===" grep -i "flutter.*version\|version.*flutter" "$file" doneLength of output: 10246
72-84: Verify the Flutter tool backend pathThe script assumes a specific Flutter root structure. Ensure this path is compatible with Flutter 3.27.1.
✅ Verification successful
The Flutter tool backend path is correctly structured
The path
${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.shfollows Flutter's standard SDK structure, as evidenced by its consistent usage across multiple Flutter projects and platforms in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the tool_backend.sh exists in the specified location rg -l "tool_backend.sh" | grep "flutter_tools"Length of output: 44
Script:
#!/bin/bash # Search for FLUTTER_ROOT variable definitions and usage rg "FLUTTER_ROOT" -A 2 -B 2 # Look for any Flutter SDK path configurations fd -t f ".*\.txt$" -X rg "flutter.*sdk|sdk.*flutter" -iLength of output: 19474
examples/wallet_app/linux/CMakeLists.txt (1)
124-128: LGTM: Conditional AOT library installationThe AOT library installation is correctly configured to exclude Debug builds.
Summary by CodeRabbit
Release Notes
Documentation
starknet-devnetusage instructions with improved phrasing for clarityBuild Configuration
Platform Support
User Interface
Maintenance