Studio: Display spinner when the site is being pulled#672
Merged
Conversation
katinthehatsite
approved these changes
Nov 19, 2024
Contributor
katinthehatsite
left a comment
katinthehatsite
left a comment
There was a problem hiding this comment.
The functionality looks good to me and works as expected 👍
Nice work!
I noticed that some unit tests failed and tried re-retriggering them. They might pass but otherwise, would need to be adjusted before merging.
Contributor
Author
|
Thank you for your review and testing, Kat.
Oh, yeah, I missed running unit tests before asking for review yesterday evening. Sorry about that. I have now fixed the unit tests in 123890c (and subsequent lint fix 5f32895). We are also waiting for feedback from Matt and Lucas now (https://github.com/Automattic/dotcom-forge/issues/9897#issuecomment-2487994378), so more changes may come in. I will share another comment here once the PR is ready for another review round. |
Contributor
Author
|
This PR is ready for another review round. |
wojtekn
approved these changes
Nov 21, 2024
Contributor
wojtekn
left a comment
wojtekn
left a comment
There was a problem hiding this comment.
Works well, and the code change looks good. I left one note regarding code readability.
2 tasks
ivan-ottinger
added a commit
that referenced
this pull request
May 13, 2026
## Related issues Reverts #3363. Restoring this once the upstream fix lands in the [`wordpress-mobile/release-toolkit`](https://github.com/wordpress-mobile/release-toolkit) plugin. ## How AI was used in this PR AI-assisted: diagnosed the post-merge failure from the Buildkite log, identified the gap in the fastlane plugin's platform allowlist, and drafted this revert. Author reviewed. ## Why we're reverting After #3363 merged, the next trunk build's `Distribute Dev Builds` step ([Buildkite #16073](https://buildkite.com/automattic/studio/builds/16073)) failed with: ``` Error setting value 'Linux - x64' for option 'platform' You passed invalid parameters to 'upload_build_to_apps_cdn'. Platform must be one of: Android, iOS, Mac - Silicon, Mac - Intel, Mac - Any, Windows - x86, Windows - x64, Windows - ARM64, Microsoft Store - x86, Microsoft Store - x64, Microsoft Store - ARM64 ``` The `upload_build_to_apps_cdn` fastlane action (from the [`wordpress-mobile/release-toolkit`](https://github.com/wordpress-mobile/release-toolkit) gem, pinned at `~> 14.2`) has a **client-side** allowlist baked into its `VALID_PLATFORMS` constant (`lib/fastlane/plugin/wpmreleasetoolkit/actions/common/upload_build_to_apps_cdn.rb:31-43`). That list doesn't include `'Linux - x64'` or `'Linux - ARM64'`, so the action rejects our call before it ever reaches WPCOM. The WPCOM-side allowlist updates (the enum + extension MIME types) are landed and correct — they handle the *server-side* validation. But the *client-side* validation in the fastlane plugin is a separate codebase that also needs to be updated. We missed that gap before merging. Impact while #3363 was on trunk: - Mac and Windows dev builds **did** upload to AppsCDN (they iterate first in the loop). - Linux upload raised, terminating the loop before the `buildkite_annotate` summary fired. - The `Distribute Dev Builds` step shows red despite Mac/Windows being in CDN — confusing for anyone watching trunk. - Every subsequent trunk merge would hit the same failure, blocking the visible distribution annotation indefinitely. Reverting restores the clean Mac/Windows distribution flow until the plugin gap is fixed upstream. ## Proposed Changes Reverts the two-file change from #3363: - `.buildkite/pipeline.yml`: drops `dev-linux` from `Distribute Dev Builds`'s `depends_on` and removes the `*.deb` artifact download. - `fastlane/Fastfile`: removes the `linux_deb_path` helper, the `release_tag.nil?` block adding Linux entries to `update_builds`, and the related comment. ## Next steps after this lands 1. Open a PR against [`wordpress-mobile/release-toolkit`](https://github.com/wordpress-mobile/release-toolkit) adding `'Linux - x64'` and `'Linux - ARM64'` to `VALID_PLATFORMS` in `upload_build_to_apps_cdn.rb`. Precedent: their PR #672 added `'Microsoft Store - x64'` and `'Windows - x64'` to the same array. 2. Wait for the plugin's next gem release. 3. Bump the `Gemfile` to that version. 4. Re-apply #3363's changes (cherry-pick or fresh PR). 5. Re-verify on the next trunk distribute. ## Testing Instructions 1. Open the next trunk build after this merges. 2. Confirm `Distribute Dev Builds` step succeeds and the `buildkite_annotate` summary lists Mac and Windows (and only those) in the "🔗 Build available for ..." message. ## Pre-merge Checklist - [ ] Have you checked for TypeScript, React or other console errors? - [x] Trunk's `Distribute Dev Builds` step succeeds again after this lands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
Proposed Changes
SyncingtooltipAdding- when a new site is being addedImporting- when an exported site is being importedLoading- fallback tooltip messageTesting Instructions
STUDIO_SITE_SYNC=true npm start.Testing other tooltip messages:
Addingtooltip when the site is being added.Importingtooltip as the site is being imported.Pre-merge Checklist