Skip to content

Handle the case where we can't identify the release. #4331

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

Merged
merged 6 commits into from
Nov 15, 2022

Conversation

kaibolay
Copy link
Contributor

Developers can use adb shell setprop debug.firebase.appdistro.devmode true to skip checking the release version (and skip actually submitting feedback - while keeping the UI).

@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 15, 2022

Coverage Report 1

Affected Products

  • firebase-appdistribution

    Overall coverage changed from ? (411cb3b) to 77.21% (aa90f51) by ?.

    40 individual files with coverage change

    FilenameBase (411cb3b)Merge (aa90f51)Diff
    AabUpdater.java?98.68%?
    ApkInstaller.java?96.88%?
    ApkUpdater.java?93.75%?
    AppDistributionReleaseImpl.java?100.00%?
    AppDistributionReleaseInternal.java?100.00%?
    AppIconSource.java?85.71%?
    AutoValue_AppDistributionReleaseImpl.java?65.45%?
    AutoValue_AppDistributionReleaseInternal.java?66.67%?
    AutoValue_ImageUtils_ImageSize.java?35.00%?
    AutoValue_TesterApiDisabledErrorDetails.java?29.41%?
    AutoValue_TesterApiDisabledErrorDetails_HelpLink.java?54.17%?
    AutoValue_UpdateProgressImpl.java?65.96%?
    ErrorMessages.java?0.00%?
    FeedbackActivity.java?1.47%?
    FeedbackSender.java?90.91%?
    FirebaseAppDistributionExceptions.java?80.00%?
    FirebaseAppDistributionFileProvider.java?0.00%?
    FirebaseAppDistributionImpl.java?89.82%?
    FirebaseAppDistributionLifecycleNotifier.java?96.67%?
    FirebaseAppDistributionNotificationsManager.java?85.00%?
    FirebaseAppDistributionRegistrar.java?82.05%?
    FirebaseAppDistributionTesterApiClient.java?87.63%?
    HttpsUrlConnectionFactory.java?50.00%?
    ImageUtils.java?100.00%?
    InstallActivity.java?2.53%?
    LogWrapper.java?68.75%?
    NewReleaseFetcher.java?77.55%?
    PackageInfoUtils.java?42.86%?
    ReleaseIdentifier.java?85.06%?
    ReleaseUtils.java?83.33%?
    ScreenshotTaker.java?62.16%?
    SignInResultActivity.java?0.00%?
    SignInStorage.java?42.86%?
    TakeScreenshotAndStartFeedbackActivity.java?0.00%?
    TaskUtils.java?96.30%?
    TesterApiDisabledErrorDetails.java?93.75%?
    TesterApiHttpClient.java?89.47%?
    TesterSignInManager.java?93.62%?
    UpdateProgressImpl.java?100.00%?
    UpdateTaskImpl.java?75.71%?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/DnObPJdqGx.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 15, 2022

Size Report 1

Affected Products

  • firebase-appdistribution

    TypeBase (411cb3b)Merge (aa90f51)Diff
    aar143 kB144 kB+977 B (+0.7%)
    apk (aggressive)820 kB822 kB+1.66 kB (+0.2%)
    apk (release)2.00 MB2.00 MB+1.83 kB (+0.1%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/Wyyi7KAbaa.html

Copy link
Contributor

@lfkellogg lfkellogg left a comment

Choose a reason for hiding this comment

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

A null release name isn't the most intuitive signal that we are in dev mode, but I see how it makes the Task flow easier in doStartFeedback, as opposed to checking for dev mode when handling the task failure.

Do you think it's worth at least making the argument to FeedbackActivity more explicit? Maybe with a boolean dev mode flag?

@kaibolay
Copy link
Contributor Author

A null release name isn't the most intuitive signal that we are in dev mode, but I see how it makes the Task flow easier in doStartFeedback, as opposed to checking for dev mode when handling the task failure.

Ack.

Do you think it's worth at least making the argument to FeedbackActivity more explicit? Maybe with a boolean dev mode flag?

I experimented with it and it was more clunky. I opted for a generous sprinkling of comments (and @Nullable) - I think that'll make it clear what is going on.

@kaibolay kaibolay requested a review from lfkellogg November 15, 2022 21:01
Copy link
Contributor

@lfkellogg lfkellogg left a comment

Choose a reason for hiding this comment

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

Looks great!

@kaibolay kaibolay merged commit c78e761 into fad/in-app-feedback Nov 15, 2022
@kaibolay kaibolay deleted the kb/handle-no-release branch November 15, 2022 23:05
lfkellogg pushed a commit that referenced this pull request Nov 21, 2022
Use `adb shell setprop debug.firebase.appdistro.devmode true` for development mode. This mode doesn't verify that a release is present and doesn't actually submit feedback - it just demonstrates the UI,
lfkellogg pushed a commit that referenced this pull request Dec 7, 2022
Use `adb shell setprop debug.firebase.appdistro.devmode true` for development mode. This mode doesn't verify that a release is present and doesn't actually submit feedback - it just demonstrates the UI,
lfkellogg pushed a commit that referenced this pull request Dec 9, 2022
Use `adb shell setprop debug.firebase.appdistro.devmode true` for development mode. This mode doesn't verify that a release is present and doesn't actually submit feedback - it just demonstrates the UI,
@firebase firebase locked and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants