Add commandline option for passing features to cargo expand during codegen #2284

fmeef · 2024-09-08T10:27:32Z

This fixes #2283

welcome · 2024-09-08T10:27:34Z

Hi! Thanks for opening this pull request! 😄

fzyzcjy

(just a quick one-line nit when skimming it)

frb_codegen/src/binary/commands.rs

fzyzcjy · 2024-09-08T15:05:28Z

Looks like there is a 10k line of code diff, possibly because some cargokit folders etc are missing. One quick way to fix this error is to revert unwanted changes in those folders.

Feel free to ping me if you need any suggestions to make CI pass etc, or ping me when it is ready for review!

fmeef · 2024-09-09T01:37:46Z

Yes, I just realized that happened when running ./frb_internal precommit --mode slow, fixing it.

codecov · 2024-09-09T05:58:43Z

Codecov Report

Attention: Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 98.96%. Comparing base (48f02f4) to head (0fa5194).
Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
..._codegen/src/library/commands/cargo_expand/real.rs	83.33%	3 Missing ⚠️
frb_codegen/src/binary/commands.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2284      +/-   ##
==========================================
- Coverage   98.98%   98.96%   -0.02%     
==========================================
  Files         489      489              
  Lines       20231    20252      +21     
==========================================
+ Hits        20025    20042      +17     
- Misses        206      210       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fzyzcjy · 2024-09-09T06:19:32Z

The code LGTM, maybe we can add a test for it. For example, maybe we can

Add a function that only compiles when a custom feature is enabled, in https://github.com/fzyzcjy/flutter_rust_bridge/blob/master/frb_example/pure_dart/rust/src/api/misc_no_twin_example_a.rs
Modify https://github.com/fzyzcjy/flutter_rust_bridge/blob/master/tools/frb_internal/lib/src/makefile_dart/generate.dart#L302 such that it provides that feature flag
We should see the function has corresponding dart function generated
You can test it by ./frb_internal generate-run-frb-codegen-command-generate --package frb_example/pure_dart

fmeef · 2024-09-09T10:14:42Z

While I have these tests added, I am trying to find a way to make linting/clippy respect the correct feature flags in a clean way. Will let you know when its fully working again

fmeef · 2024-09-14T10:27:30Z

Appears to be fully working again with the tests

fzyzcjy

Great job! Mainly some nits :)

frb_codegen/src/binary/commands.rs

frb_codegen/src/library/codegen/parser/internal_config.rs

frb_codegen/src/library/codegen/parser/mod.rs

frb_codegen/src/library/commands/cargo_expand/real.rs

frb_example/pure_dart/build.dart

frb_utils/lib/src/simple_build_utils.dart

tools/frb_internal/lib/src/makefile_dart/consts.dart

tools/frb_internal/lib/src/makefile_dart/generate.dart

tools/frb_internal/lib/src/makefile_dart/misc.dart

tools/frb_internal/lib/src/makefile_dart/release.dart

fmeef · 2024-09-15T23:26:40Z

Do these latest changes work so far?

fzyzcjy · 2024-09-16T00:33:31Z

Looks great! I will review again within a day. Btw next time when you are ready, feel free to use "request a review" button or tell me.

fmeef · 2024-09-16T00:39:13Z

Oh yes sorry will use that next time

fzyzcjy · 2024-09-16T00:46:12Z

It's OK!

fzyzcjy · 2024-09-16T08:27:56Z

Reviewed half, and then realize it seems that the discussions in

are not solved. Feel free to ping me after these are handled as well!

tools/frb_internal/lib/src/makefile_dart/consts.dart

tools/frb_internal/lib/src/makefile_dart/generate.dart

fzyzcjy · 2024-09-18T14:39:49Z

The progress looks good, and feel free to ping me when the things above are handled!

fmeef · 2024-09-19T09:22:18Z

seems to be ready

fzyzcjy · 2024-09-19T10:46:55Z

Great! I will review probably within a day

fzyzcjy

Great! Ready to merge after nits

tools/frb_internal/lib/src/makefile_dart/consts.dart

tools/frb_internal/lib/src/makefile_dart/misc.dart

tools/frb_internal/lib/src/makefile_dart/lint.dart

frb_example/pure_dart/rust/src/api/misc_no_twin_example_a.rs

fzyzcjy

Great! I am ready to merge and only one missing nit (other two can be fixed by using github's button)

frb_utils/lib/src/dart_web_test_utils/run_test.dart

tools/frb_internal/lib/src/makefile_dart/lint.dart

fzyzcjy · 2024-09-20T14:45:02Z

@all-contributors please add @fmeef for code

allcontributors · 2024-09-20T14:45:12Z

@fzyzcjy

I've put up a pull request to add @fmeef! 🎉

frb_utils/lib/src/commands/test_web_command.dart

fmeef

Fixed

welcome · 2024-09-21T03:40:45Z

Hi! Congrats on merging your first pull request! 🎉

fzyzcjy mentioned this pull request Sep 8, 2024

Passing feature flags to flutter_rust_bridge_codegen is not possible #2283

Closed

fzyzcjy reviewed Sep 8, 2024

View reviewed changes

frb_codegen/src/binary/commands.rs Outdated Show resolved Hide resolved

fmeef force-pushed the cargo_features branch from 69328d2 to c73f805 Compare September 9, 2024 02:17

fmeef force-pushed the cargo_features branch 5 times, most recently from f418c86 to 319e2f7 Compare September 9, 2024 10:05

fmeef force-pushed the cargo_features branch 12 times, most recently from 1e26146 to 038620e Compare September 10, 2024 09:20

fzyzcjy force-pushed the master branch from 09a79ef to 870e5fc Compare September 11, 2024 00:45

fmeef force-pushed the cargo_features branch from 038620e to e3373e2 Compare September 14, 2024 08:34

fzyzcjy reviewed Sep 14, 2024

View reviewed changes

fzyzcjy reviewed Sep 16, 2024

View reviewed changes

tools/frb_internal/lib/src/makefile_dart/consts.dart Outdated Show resolved Hide resolved

fzyzcjy reviewed Sep 16, 2024

View reviewed changes

tools/frb_internal/lib/src/makefile_dart/generate.dart Outdated Show resolved Hide resolved

fmeef force-pushed the cargo_features branch from c7b0a53 to 4f4acc0 Compare September 18, 2024 07:39

fmeef force-pushed the cargo_features branch from 4f4acc0 to b77625c Compare September 19, 2024 07:41

fzyzcjy reviewed Sep 19, 2024

View reviewed changes

fmeef force-pushed the cargo_features branch 3 times, most recently from 2bc411e to 26f152b Compare September 20, 2024 09:54

fzyzcjy approved these changes Sep 20, 2024

View reviewed changes

frb_utils/lib/src/dart_web_test_utils/run_test.dart Outdated Show resolved Hide resolved

tools/frb_internal/lib/src/makefile_dart/lint.dart Outdated Show resolved Hide resolved

tools/frb_internal/lib/src/makefile_dart/lint.dart Outdated Show resolved Hide resolved

allcontributors bot mentioned this pull request Sep 20, 2024

docs: add fmeef as a contributor for code #2306

Merged

fmeef force-pushed the cargo_features branch 2 times, most recently from a466ffa to 16f57c4 Compare September 21, 2024 00:09

fzyzcjy reviewed Sep 21, 2024

View reviewed changes

frb_utils/lib/src/commands/test_web_command.dart Outdated Show resolved Hide resolved

fmeef force-pushed the cargo_features branch from 16f57c4 to 1aa537e Compare September 21, 2024 00:45

Add option to select feature flags for codegen

518c0ba

fmeef force-pushed the cargo_features branch from 1aa537e to 518c0ba Compare September 21, 2024 00:51

fmeef commented Sep 21, 2024

View reviewed changes

fzyzcjy merged commit 134374e into fzyzcjy:master Sep 21, 2024
135 checks passed

fzyzcjy mentioned this pull request Sep 27, 2024

rust_features unknown field #2324

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add commandline option for passing features to cargo expand during codegen #2284

Add commandline option for passing features to cargo expand during codegen #2284

fmeef commented Sep 8, 2024

welcome bot commented Sep 8, 2024

fzyzcjy left a comment

fzyzcjy commented Sep 8, 2024

fmeef commented Sep 9, 2024

codecov bot commented Sep 9, 2024 •

edited

Loading

fzyzcjy commented Sep 9, 2024

fmeef commented Sep 9, 2024

fmeef commented Sep 14, 2024

fzyzcjy left a comment

fmeef commented Sep 15, 2024

fzyzcjy commented Sep 16, 2024

fmeef commented Sep 16, 2024

fzyzcjy commented Sep 16, 2024

fzyzcjy commented Sep 16, 2024

fzyzcjy commented Sep 18, 2024 •

edited

Loading

fmeef commented Sep 19, 2024

fzyzcjy commented Sep 19, 2024

fzyzcjy left a comment

fzyzcjy left a comment

fzyzcjy commented Sep 20, 2024

allcontributors bot commented Sep 20, 2024

fmeef left a comment

welcome bot commented Sep 21, 2024

Add commandline option for passing features to cargo expand during codegen #2284

Add commandline option for passing features to cargo expand during codegen #2284

Conversation

fmeef commented Sep 8, 2024

welcome bot commented Sep 8, 2024

fzyzcjy left a comment

Choose a reason for hiding this comment

fzyzcjy commented Sep 8, 2024

fmeef commented Sep 9, 2024

codecov bot commented Sep 9, 2024 • edited Loading

Codecov Report

fzyzcjy commented Sep 9, 2024

fmeef commented Sep 9, 2024

fmeef commented Sep 14, 2024

fzyzcjy left a comment

Choose a reason for hiding this comment

fmeef commented Sep 15, 2024

fzyzcjy commented Sep 16, 2024

fmeef commented Sep 16, 2024

fzyzcjy commented Sep 16, 2024

fzyzcjy commented Sep 16, 2024

fzyzcjy commented Sep 18, 2024 • edited Loading

fmeef commented Sep 19, 2024

fzyzcjy commented Sep 19, 2024

fzyzcjy left a comment

Choose a reason for hiding this comment

fzyzcjy left a comment

Choose a reason for hiding this comment

fzyzcjy commented Sep 20, 2024

allcontributors bot commented Sep 20, 2024

fmeef left a comment

Choose a reason for hiding this comment

welcome bot commented Sep 21, 2024

codecov bot commented Sep 9, 2024 •

edited

Loading

fzyzcjy commented Sep 18, 2024 •

edited

Loading