Skip to content

Conversation

@cacieprins
Copy link
Contributor

@cacieprins cacieprins commented Oct 7, 2025

  • Closes

Additional details

  • migrates trigger-publish-binary-pipeline.js to an inlined bash script to speed it up and simplify it
  • adds a pipeline parameter that can be set via CircleCI UI to determine which branch we use in the publish-binary pipeline, to reduce file churn and human error
  • updates the packages/electron/README.md to include the simplified steps

This should reduce cognitive load regarding branches when performing Electron upgrades.

Steps to test

How has the user experience changed?

PR Tasks


Note

Adds a CircleCI parameter to select the cypress-publish-binary branch and replaces the Node trigger script with an inline bash command, updating docs accordingly.

  • CI (CircleCI)
    • Add pipeline parameter publish-binary-branch in .circleci/config.yml and @pipeline.yml.
    • Replace Node trigger script with new command commands/trigger-publish-binary-pipeline.yml (bash), persisting triggered_pipeline.json.
    • Wire command into create-and-trigger-packaging-artifacts job, passing << pipeline.parameters.publish-binary-branch >>.
    • Remove scripts/binary/trigger-publish-binary-pipeline.js.
  • Docs
    • Update packages/electron/README.md Electron upgrade steps to include adding branch to &full-workflow-filters and setting publish-binary-branch in CircleCI UI.

Written by Cursor Bugbot for commit 2b684c4. This will update automatically on new commits. Configure here.

…ed value for branch

- migrates trigger-publish-binary-pipeline.js to an inlined bash script to speed
  it up and simplify it
- adds a pipeline parameter that can be set via CircleCI UI to determine which
  branch we use in the publish-binary pipeline, to reduce file churn and human
  error
- updates the packages/electron/README.md to include the simplified steps
- [ ] Temporarily update the circle configuration to allow `cypress` to run against the branch
- [ ] Temporarily set target `cypress-publish-binary` branch as a `branch` property on the request body in [../../scripts/binary/trigger-publish-binary-pipeline.js](../../scripts/binary/trigger-publish-binary-pipeline.js) script, so that you can test against this branch from the electron upgrade branch. This property must be set both at the root of the body object, and on the `parameters` key. If it is not set at the root, the binary pipeline will continue to use the primary branch.
- [ ] Add your branch name to the `&full-workflow-filters` anchor in [`@pipeline.yml`](../../.circleci/src/pipeline/@pipeline.yml)
Copy link
Contributor Author

@cacieprins cacieprins Oct 7, 2025

Choose a reason for hiding this comment

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

this can be parameterized as well (separately, to reduce delta surface)

@cacieprins cacieprins self-assigned this Oct 7, 2025
@cacieprins cacieprins marked this pull request as ready for review October 10, 2025 14:32
@cacieprins cacieprins marked this pull request as draft October 10, 2025 14:33
cursor[bot]

This comment was marked as outdated.

@cacieprins cacieprins marked this pull request as ready for review October 10, 2025 14:39
cursor[bot]

This comment was marked as outdated.

@AtofStryker AtofStryker self-requested a review October 10, 2025 15:22
@cypress
Copy link

cypress bot commented Oct 10, 2025

cypress    Run #67039

Run Properties:  status check passed Passed #67039  •  git commit 2b684c4bfa: Merge branch 'develop' into ci/parameterize-binary-branch
Project cypress
Branch Review ci/parameterize-binary-branch
Run status status check passed Passed #67039
Run duration 19m 17s
Commit git commit 2b684c4bfa: Merge branch 'develop' into ci/parameterize-binary-branch
Committer Cacie Prins
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 14
Tests that did not run due to a developer annotating a test with .skip  Pending 1098
Tests that did not run due to a failure in a mocha hook  Skipped 4
Tests that passed  Passing 26522
View all changes introduced in this branch ↗︎

Warning

Partial Report: The results for the Application Quality reports may be incomplete.

UI Coverage  45.61%
  Untested elements 187  
  Tested elements 161  
Accessibility  97.98%
  Failed rules  4 critical   8 serious   2 moderate   2 minor
  Failed elements 101  

cursor[bot]

This comment was marked as outdated.

@cacieprins cacieprins force-pushed the ci/parameterize-binary-branch branch from 57afea5 to 5d50a15 Compare October 14, 2025 18:16
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

@cacieprins Is it possible to have the trigger-publish-binary-pipeline workflow run on this PR to test it working?

if [[ $? != 0 ]]; then
echo "Failed to compose the request body. Skipping pipeline trigger."
exit 1
fi
Copy link

Choose a reason for hiding this comment

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

Bug: Bug

The JSON body for the pipeline trigger directly interpolates environment variables like $CIRCLE_BRANCH without proper JSON escaping. If these variables contain special characters, the JSON becomes malformed, causing the API request to fail.

Fix in Cursor Fix in Web

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.

4 participants