fix: pin @sourceacademy/conductor to a published version everywhere#795
fix: pin @sourceacademy/conductor to a published version everywhere#795Akshay-2007-1 wants to merge 3 commits into
Conversation
repeat and testplugin depended on conductor via a bare, unpinned GitHub URL. yarn.lock had resolved and locked that to a commit from before even the BaseModulePlugin binding fix (conductor#41) - a real `yarn install` builds against that stale, broken conductor entirely silently, since nothing forces Yarn to re-resolve an already-locked git dependency. Other bundles (rune, etc.) already depend on the versioned npm release; switched these two to match (^0.7.0), which also resolves a duplicate-package TS error that showed up in any bundle depending on both specs simultaneously.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the dependency @sourceacademy/conductor from a direct GitHub repository reference to the npm package version ^0.7.0 in lib/testplugin/package.json, src/bundles/repeat/package.json, and yarn.lock. I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
This should be done using the |
…nductor-dependency-pin Only yarn.lock conflicted; repeat's and testplugin's package.json edits (pinning @sourceacademy/conductor to a published version) merged cleanly since conductor-migration's own copies of those files don't touch that dependency. Also addresses LeeYi's review comment: added @sourceacademy/conductor to the yarn catalog (.yarnrc.yml) and switched both package.json's pins to "catalog:", matching the convention already used for typescript/js-slang/react etc. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Addressed - added (Note: I used the plain Also merged in |
RichDom2185
left a comment
RichDom2185
left a comment
There was a problem hiding this comment.
There seem to be unintentional dependency downgrades in yarn.lock – please reset it to the current state using git checkout origin/conductor-migration -- yarn.lock and rerun yarn install to make sure only conductor is updated
- Add @sourceacademy/conductor to npmPreapprovedPackages to bypass the age gate - Reset yarn.lock to base state and rerun yarn install so only conductor's resolution changes, undoing unintentional dependency downgrades Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixed in 1353241, reset yarn.lock to origin/conductor-migration and reran yarn install |
…igrate-sound Resolved conflicts by keeping the already-Conductor-migrated code (midi, sound, and stereo_sound's retirement) over conductor-migration's stale pre-migration content, same pattern as feat/migrate-binary-tree and feat/migrate-midi. Also re-applies the noImplicitOverride fix and the midi scale-test fix that landed separately on feat/migrate-midi (this branch has its own independent midi history). yarn.lock resolved per the review feedback on PR #795 (reset to origin/conductor-migration's copy first, then reinstalled, so only new/changed dependencies actually move instead of picking up whatever was stale on this branch); .yarnrc.yml also gets @sourceacademy/conductor added to npmPreapprovedPackages for the same reason as PR #795. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#795 (fix/conductor-dependency-pin) is about to merge into conductor-migration first, establishing @sourceacademy/conductor in the yarn catalog and npmPreapprovedPackages. Switching midi/sound/ tab-Sound's package.json over to "catalog:" now (matching #795 verbatim) avoids re-doing this exact reconciliation the next time this branch merges conductor-migration in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches the same fix already applied on #795 and #796: adds @sourceacademy/conductor to the yarn catalog and npmPreapprovedPackages, and switches midi's own package.json to "catalog:" instead of a literal "^0.7.0" pin, so this PR doesn't leave midi on the old convention once #795 lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 participants
Summary
Test plan