Skip to content

don't disable cloud sync for skillmap#11008

Merged
riknoll merged 1 commit into
masterfrom
dev/riknoll/arcade-fix-skillmap-cloud-sync
Dec 11, 2025
Merged

don't disable cloud sync for skillmap#11008
riknoll merged 1 commit into
masterfrom
dev/riknoll/arcade-fix-skillmap-cloud-sync

Conversation

@riknoll
Copy link
Copy Markdown
Member

@riknoll riknoll commented Dec 11, 2025

this was regressed by #9546. I believe that change went out in arcade in march, so it's been broken for most of this year.

fixes cloud syncing of skillmap projects.

@riknoll riknoll requested a review from a team December 11, 2025 22:42
@abchatra
Copy link
Copy Markdown
Collaborator

Now I question the popularity of skillmap

@abchatra
Copy link
Copy Markdown
Collaborator

We should hotfix this right away.

Copy link
Copy Markdown
Member

@jwunderl jwunderl left a comment

Choose a reason for hiding this comment

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

it looks like that enableAuth(false) path was hit prior to that pr, that pr bumped it up a lil earlier but the original intention of the check was skipping for skillmap? https://github.com/microsoft/pxt/pull/7934/changes

or is there something else in between I'm missing there, like auth being enabled until workspace is set up then disabled after? either way change seems fine now just curious

@riknoll
Copy link
Copy Markdown
Member Author

riknoll commented Dec 11, 2025

@jwunderl that linked pr is super old. if you look at the file prior to the change i linked, the check for enabling auth was looking to see if we are using the iframe workspace. the skillmap uses the browser workspace (see the ws=browser part of the url here)

@riknoll
Copy link
Copy Markdown
Member Author

riknoll commented Dec 11, 2025

the workspace was changed here #8290

@riknoll riknoll merged commit 4e02d07 into master Dec 11, 2025
20 checks passed
@riknoll riknoll deleted the dev/riknoll/arcade-fix-skillmap-cloud-sync branch December 11, 2025 23:18
riknoll added a commit that referenced this pull request Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants