getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
582 stars 207 forks source link

Dirty check fails after Cocoa version bump #2595

Open jamescrosswell opened 1 year ago

jamescrosswell commented 1 year ago

We have a chore(deps) PR that bumps the CocoaSDK from v8.10.0 to v8.11.0.

It looks like although this bumps the CocoaSDK, it hasn't recompiled our bindings and committed the updated version of these to source (which is why the dirty check is failing when CI runs the build on this PR).

image

I'm not sure how the chore(deps) PRs get created (presumably by some kind of bot) but however it happens, we need to tweak the process so that it not only bumps the version of the Sentry Cocoa module we depend on but also rebuilds our bindings and commits any changes to thes as part of the same PR, each time we upgrade this module.

jamescrosswell commented 3 months ago

The cocoa version bump itself is triggered here: https://github.com/getsentry/sentry-dotnet/blob/d32f05ba46ca3a022224dae039b27f13691ac06c/.github/workflows/update-deps.yml#L16-L18

Which runs this custom workflow: https://github.com/getsentry/sentry-dotnet/blob/d32f05ba46ca3a022224dae039b27f13691ac06c/.github/workflows/update-deps.yml#L25-L28

Currently that workflow runs update-dependency.ps1, which essentially just updates the submodule, creates a commit and then creates a PR for this commit.

In the case of the Cocoa SDK, where submodule changes might result in changes to src/Sentry.Bindings.Cocoa/ApiDefinitions.cs, we need to rebuild the submodule as part of preparing the PR.

Ideas

bitsandfoxes commented 3 months ago

We're going to have to figure out how to deal with the swift headers before tackling this. Right now, I've got to:

  1. Run scripts/generate-cocoa-bindings.ps1
  2. Check what changed on the API bindings
  3. Take what made it to the bindings from the swift header
  4. Run scripts/generate-cocoa-bindings.ps1 again
  5. Check that stuff in.

As context: We "have" to do this because SentryId got moved into the swift headers and now we "have" to import that header too when generating the binding. We could "mock" that one type tho.