Skip to content
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

CI: Check Dart/Flutter bindings & run static code analysis for Dart/Flutter packages #161

Merged
merged 15 commits into from
May 6, 2024

Conversation

erdemyerebasmaz
Copy link
Contributor

@erdemyerebasmaz erdemyerebasmaz commented May 3, 2024

CI workflows for pull requests will be

  • checking changes in Dart/Flutter bindings &
  • running static code analysis for Dart/Flutter packages

with this PR.

Other changes:
  • Added just all recipe(command) that bundles the whole process 55e1103
  • List available recipes by default cbf47f7
  • symlink files for C headers are removed & are now ignored by version control 8ec6e70
  • Fix the path for generated Dart docs 79a5069
  • Default line length is set to 80(default) for all Dart files across the repository, down from preferred 110
    • There are no options to configure line length on files generated by ffigen and there are no options to exclude files from dart format, which conflicted with checking formatting differences step on CI.
      A workaround will be addressed on a separate PR as it'll involve folder restructuring & explicit format commands for all packages & melos scripts.
  • Update flutter_rust_bridge to version 2.0.0-dev.33
    • Regenerated Dart/Flutter bindings
  • Update Dart & Flutter SDK ranges
  • Update dependencies to latest and increase dependency ranges to support frb ">=2.0.0-dev.0 <=2.0.0-dev.33"
  • Export error structs through Dart bindings
Known issues:

flutter_rust_bridge_codegen generate emits a SEVERE error which should be ignored. This issue will be reported on frb repository.

[2024-05-04T17:49:02.994Z WARN /home/yse/.cargo/registry/src/index.crates.io-6f17d22bba15001f/flutter_rust_bridge_cod
egen-2.0.0-dev.32/src/library/commands/ffigen.rs:97] The `ffigen` command emitted a SEVERE error. Maybe there is a pr
oblem? Please refer to https://fzyzcjy.github.io/flutter_rust_bridge/manual/ffigen-troubleshooting for details
Logs containing SEVERE:
[SEVERE] : Unknown key - 'ignore-source-errors'.

Addressed by continuing if there are any errors when generating bindings.

@erdemyerebasmaz erdemyerebasmaz changed the title CI: Check Dart/Flutter bindings & run static code analysis for Dart/Flutter packages [Draft] CI: Check Dart/Flutter bindings & run static code analysis for Dart/Flutter packages May 3, 2024
@erdemyerebasmaz erdemyerebasmaz force-pushed the flutter_ci branch 4 times, most recently from 2ab2c43 to fd298a6 Compare May 5, 2024 09:46
@erdemyerebasmaz erdemyerebasmaz changed the title [Draft] CI: Check Dart/Flutter bindings & run static code analysis for Dart/Flutter packages CI: Check Dart/Flutter bindings & run static code analysis for Dart/Flutter packages May 5, 2024
- Add static analysis step for Dart & Flutter packages
- Ignore warning for unused element on build scrips for Windows targets
- Continue if there are any errors with generating bindings
Remove symlink files from version control
@erdemyerebasmaz
Copy link
Contributor Author

erdemyerebasmaz commented May 6, 2024

Recent changes:

  • Rename all recipe to bootstrap
    • Added frb parameter to install or skip installing fluttter_rust_bridge dependencies
  • Disable test-flutter recipe as Flutter packages currently does not have any integration tests
  • Do not build library again on child recipes of test
    • test-dart & test-flutter were building the library as first step, if these are called from the wrapper test recipe, they do not build the library
  • Fixed file path & profile for dynamic library used on Dart package tests
    • Added a dummy test to Dart package

Update README accordingly
 - Remove Melos from prerequisites
Update dependencies to latest & increase the supported range
Copy link
Collaborator

@dangeross dangeross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@erdemyerebasmaz erdemyerebasmaz merged commit cc85881 into main May 6, 2024
0 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants