Skip to content

Conversation

nbradbury
Copy link
Contributor

As mentioned here, using a Job for the Jetpack connection flow doesn't make sense.

The original intent was to use a Job so the flow could be cancellable, but there's really no way to cancel any of the steps. So this PR removes the Job entirely.

@dangermattic
Copy link
Collaborator

dangermattic commented Aug 20, 2025

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@nbradbury nbradbury requested a review from Copilot August 20, 2025 09:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the unused Job from the Jetpack REST connection flow, simplifying the implementation by eliminating unnecessary job management. The original intent was to make the flow cancellable, but since there's no practical way to cancel individual steps, the Job was redundant.

  • Removed Job import and related job management code
  • Renamed methods from "job" to "flow" terminology for clarity
  • Updated logging messages to reflect the change from "job" to "flow"

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 20, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr22153-b565a29
Commitb565a29
Direct Downloadwordpress-prototype-build-pr22153-b565a29.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 20, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr22153-b565a29
Commitb565a29
Direct Downloadjetpack-prototype-build-pr22153-b565a29.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Aug 20, 2025

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.13%. Comparing base (2dbfa42) to head (b565a29).
⚠️ Report is 2 commits behind head on trunk.

Files with missing lines Patch % Lines
...ckrestconnection/JetpackRestConnectionViewModel.kt 0.00% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #22153   +/-   ##
=======================================
  Coverage   39.13%   39.13%           
=======================================
  Files        2157     2157           
  Lines      102709   102709           
  Branches    15773    15773           
=======================================
  Hits        40196    40196           
  Misses      58938    58938           
  Partials     3575     3575           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nbradbury nbradbury marked this pull request as ready for review August 20, 2025 10:34
@nbradbury nbradbury requested a review from adalpari August 20, 2025 15:21
Copy link

Copy link
Contributor

@adalpari adalpari left a comment

Choose a reason for hiding this comment

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

Nice clean up, LGTM!

@nbradbury nbradbury merged commit 07b754d into trunk Aug 20, 2025
26 checks passed
@nbradbury nbradbury deleted the issue/jp-connect-remove-job branch August 20, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants