Set up CI for bob / rn compatibility matrix and handle non-standard codegen output dirs #163

Johennes · 2024-11-13T07:57:11Z

No description provided.

jhugman · 2024-11-14T16:35:39Z

Quick drive by, I know it's in draft, just a quick note to say:

This is stunning. It's going to be so helpful. Thank you.

Johennes · 2024-11-15T14:24:33Z

I think this is in a state where it can be reviewed now.

I thought a bit about adding further dimensions to the test matrix such as org-scoped vs. non-org-scoped package name and standard vs. custom codegen output paths. However, these would make the number of combinations and CI jobs soar.

Instead I just used bob version, RN version and platform for the test matrix dimensions and always use an org-scoped package name and custom codegen output paths. The thinking there is that if e.g. the job succeeds with custom codegen paths, it is likely to also succeed with the default ones whereas the opposite is not true.

The jobs currently all fail because of #161.

I would suggest to also run the workflow on PRs (as it is configured right now) but to not make it block the PR. This can be achieved by tweaking the required status checks in the repository settings.

jhugman

This is really nice. Thank you.

I will let you be the arbiter of ordering you land it in. Getting it all in order with #161 may need the build to break for a little bit, but I'm okay with that.

I thought a bit about adding further dimensions to the test matrix such as org-scoped vs. non-org-scoped package name and standard vs. custom codegen output paths. However, these would make the number of combinations and CI jobs soar.

Yes, agreed. There are some lighter weight checks in test-turbo-modules.sh which might be worth separating out in another PR. These just make sure the identifiers inn the ubrn generated files match those in the builder-bob versions.

jhugman · 2024-11-19T18:16:25Z

README.md

@@ -1,4 +1,5 @@
 [![CI](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/ci.yml/badge.svg)](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/ci.yml)
+[![build-bob compatibility](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/compat.yml/badge.svg)](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/compat.yml)


jhugman · 2024-11-19T18:17:57Z

scripts/test-turbo-modules.sh

-    error "No available iOS simulator found"
-  fi
-
-  xcodebuild -workspace "${IOS_NAME}Example.xcworkspace" -scheme "${IOS_NAME}Example" -configuration Debug -destination "id=$udid" || error "Failed to build iOS example"


Oh, I see how you're getting rid of IOS_NAME now.

jhugman · 2024-11-19T18:18:28Z

scripts/test-turbo-modules.sh

+  if [ -f "$PKG_JSON_MIXIN" ] ; then
+    jq -s '.[0] * .[1]' ./package.json "$PKG_JSON_MIXIN" > ./package.json.new
+    mv ./package.json.new ./package.json
+  fi


scripts/test-turbo-modules.sh

jhugman · 2024-11-19T18:20:43Z

.github/workflows/compat.yml

+        bob-version:
+          - 0.42.2
+          - 0.42.3
+          - latest
+        rn-version:
+          - 0.76.0
+          - latest


I love this.

.github/workflows/compat.yml

Johennes · 2024-11-20T11:46:12Z

I will let you be the arbiter of ordering you land it in. Getting it all in order with #161 may need the build to break for a little bit, but I'm okay with that.

Thanks. I guess since this is fairly well isolated, I'll let it sit for a while in hopes that I can figure out #165 and then merge both together.

Fixes: #161

Johennes · 2024-11-26T12:37:59Z

crates/ubrn_cli/src/config/npm.rs

+    fn default() -> Self {
+        Self {
+            ios: default_ios_codegen_output_dir(),
+            android: default_android_codegen_output_dir(),


It feels slightly awkward having to pass in the default values here and in the serde macros. But I think we need to do it this way to account for either the whole object missing or only one of its fields?

Johennes · 2024-11-26T12:45:23Z

@jhugman I've merged the other PR into this one after addressing your feedback from there. Sorry for the multi-PR spam. 🙈

Johennes · 2024-11-26T13:23:52Z

Hm, I'm unsure why the integration tests are failing now. 🤔

Running fixture coverall2
Running cd "/home/runner/work/uniffi-bindgen-react-native/uniffi-bindgen-react-native/fixtures/coverall2/tests/bindings" && "/home/runner/work/uniffi-bindgen-react-native/uniffi-bindgen-react-native/node_modules/.bin/tsc" "--outdir" "/home/runner/work/uniffi-bindgen-react-native/uniffi-bindgen-react-native/build/js/tsc/test_coverall2"
test_coverall2.ts(120,49): error TS2554: Expected 0-1 arguments, but got 3.

It's failing on this line but I haven't actually touched that part.

https://github.com/jhugman/uniffi-bindgen-react-native/blob/main/fixtures/coverall2/tests/bindings/test_coverall2.ts#L120

jhugman

I think if you land this now and we can fix #167 separately.

Good work, @Johennes this is a really helpful PR.

Johennes added 11 commits November 13, 2024 08:56

Set up CI for bob / rn compatibility matrix

1a9059e

Shorten job name

3c62eb6

Further shorten names

54e1b43

Appease prettier

06a92cc

Add missing \

2419811

Allow mutable yarn installs

d616f84

Install cargo-ndk if needed

5d6c630

Fix if statement

f91d93e

Install rust toolchains

6df9a36

Install Java 17

fd0c748

Appease prettier once more

580acb8

Johennes mentioned this pull request Nov 13, 2024

Doesn't work anymore with create-react-native-library@0.41.2 & react-native@0.76.1 #161

Closed

Johennes added 2 commits November 13, 2024 11:04

Add further bob versions

003c812

Add bob 0.42.1

b408f62

Johennes added 12 commits November 15, 2024 13:57

Remove 0.42.1

257884d

Remove unused --ios-name parameter

299e25d

Allow package.json mixins

90bbaca

Add package.json mixin to test matrix

3799a4f

Change fixtures path

3672ef4

Fix empty mixin name

0b08caa

Always use package.json mixin

88fb39c

Revert to standard paths and org scope

8fd9c2d

Add missing slash

125cf24

Switch back to custom codegen paths

0cdd977

Add badge

2016643

Use 'latest'

d75dc6f

Johennes marked this pull request as ready for review November 15, 2024 14:17

Johennes requested a review from jhugman November 15, 2024 14:24

Johennes mentioned this pull request Nov 18, 2024

Handle non-standard codegen output dirs #165

Merged

jhugman approved these changes Nov 19, 2024

View reviewed changes

Rename PKG_JSON_MIXIN

f233d50

Johennes added 2 commits November 25, 2024 08:27

Document cron expression

bf3e7b9

Handle non-standard codegen output dirs (#165)

59bc61e

Fixes: #161

Johennes commented Nov 26, 2024

View reviewed changes

Johennes requested a review from jhugman November 26, 2024 12:44

jhugman mentioned this pull request Nov 27, 2024

test_coverall2.ts(120,49): error TS2554: Expected 0-1 arguments, but got 3. #167

Closed

jhugman approved these changes Nov 27, 2024

View reviewed changes

Johennes changed the title ~~Set up CI for bob / rn compatibility matrix~~ Set up CI for bob / rn compatibility matrix and handle non-standard codegen output dirs Nov 28, 2024

Johennes merged commit 2d0e12c into main Nov 28, 2024
16 of 17 checks passed

Johennes deleted the johannes/compatibility-matrix branch November 28, 2024 07:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up CI for bob / rn compatibility matrix and handle non-standard codegen output dirs #163

Set up CI for bob / rn compatibility matrix and handle non-standard codegen output dirs #163

Johennes commented Nov 13, 2024

jhugman commented Nov 14, 2024

Johennes commented Nov 15, 2024

jhugman left a comment

jhugman Nov 19, 2024

jhugman Nov 19, 2024

jhugman Nov 19, 2024

jhugman Nov 19, 2024

Johennes commented Nov 20, 2024

Johennes Nov 26, 2024

Johennes commented Nov 26, 2024

Johennes commented Nov 26, 2024

jhugman left a comment

		@@ -1,4 +1,5 @@
		[![CI](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/ci.yml/badge.svg)](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/ci.yml)
		[![build-bob compatibility](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/compat.yml/badge.svg)](https://github.com/jhugman/uniffi-bindgen-react-native/actions/workflows/compat.yml)

Set up CI for bob / rn compatibility matrix and handle non-standard codegen output dirs #163

Set up CI for bob / rn compatibility matrix and handle non-standard codegen output dirs #163

Conversation

Johennes commented Nov 13, 2024

jhugman commented Nov 14, 2024

Johennes commented Nov 15, 2024

jhugman left a comment

Choose a reason for hiding this comment

jhugman Nov 19, 2024

Choose a reason for hiding this comment

jhugman Nov 19, 2024

Choose a reason for hiding this comment

jhugman Nov 19, 2024

Choose a reason for hiding this comment

jhugman Nov 19, 2024

Choose a reason for hiding this comment

Johennes commented Nov 20, 2024

Johennes Nov 26, 2024

Choose a reason for hiding this comment

Johennes commented Nov 26, 2024

Johennes commented Nov 26, 2024

jhugman left a comment

Choose a reason for hiding this comment