Don't pass CrashReporter to RustLog

bendk commented 1 year ago

RustLog is no longer using the crash reporter, since crash reports now go through the rusterrors module.

Pull Request checklist

[ ] Tests: This PR includes thorough tests or an explanation of why it does not
[ ] Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
[ ] Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

QA

[x] QA Needed

To download an APK when reviewing a PR (after all CI tasks finished running):

Click on Checks at the top of the PR page.
Click on the firefoxci-taskcluster group on the left to expand all tasks.
Click on the build-debug task.
Click on View task in Taskcluster in the new DETAILS section.
The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

Used by GitHub Actions.

csadilek commented 1 year ago

Thanks @bendk. I think this is safe to land right now and not waiting for an upgrade? If so, we should land it so you don't have to re-open the PR post Monorepo migration.

bendk commented 1 year ago

Yes, I think this should be safe to land now:

The android-components code already allows this to be nullable
That component passes the CrashReporter to ForwardOnLog, which just ignores it.

Merging before the monorepo migration sounds great to me. Tell me if there's anything I need to do to make this happen.

csadilek commented 1 year ago

@bendk OK, just add the needs-landing label, which I did now. Thanks for confirming this is safe. However, as the A-C ForwardOnLog ignores the parameter anyway, we can probably remove it?

bendk commented 1 year ago

@bendk OK, just add the needs-landing label, which I did now. Thanks for confirming this is safe. However, as the A-C ForwardOnLog ignores the parameter anyway, we can probably remove it?

I was planning on removing that when we introduce a new a-s log forwarder (https://github.com/mozilla-mobile/firefox-android/pull/728). I hope to be merging that one soon into a-s, although there are some questions about the preformance. If we don't end up merging it, I can make a standalone PR to remove the param.

mozilla-mobile / fenix