Skip to content

[CI] PR workflows + pin actions - #293

Merged
sarjevane merged 1 commit into
mainfrom
sg/pr-workflow-run
Sep 9, 2026
Merged

sarjevane merged 1 commit into
mainfrom
sg/pr-workflow-run

Conversation

@sarjevane

@sarjevane sarjevane commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Description

  • Refactor workflows to run as much as possible in PRs from forks
  • Update and pin all actions
  • Migrate Gitleaks scan from action to binary

Remaining Tasks

Related PRs in other projects (PASQAL developers only)

Additional merge criteria

Breaking changes

Checklist

  • The title of the PR follows the right format: [{Label}] {Short Message}. Label examples: IMPROVEMENT, FIX, REFACTORING... Short message is about what your PR changes.

Versioning (PASQAL developers only)

  • Update the version of pasqal-cloud in VERSION.txt following the changes in your PR and by using semantic versioning.

Documentation

  • Update CHANGELOG.md with a description explaining briefly the changes to the users.

Tests

  • Unit tests have been added or adjusted.
  • Tests were run locally.

Internal tests pipeline (PASQAL developers only)

  • Update and run the internal tests while targeting the branch of this PR.
    If your PR hasn't changed any functionality, it still needs to be validated against internal tests.

After updating the version (PASQAL developers only)

  • Open a PR on the internal tests that updates the version used for the pasqal-cloud backward compatibility tests.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-09 12:57 UTC

Comment thread .github/workflows/verify-if-secrets-were-leaked.yml
@sarjevane
sarjevane merged commit c55e451 into main Sep 9, 2026
8 checks passed
@sarjevane
sarjevane deleted the sg/pr-workflow-run branch September 9, 2026 12:57

@Fenkiou Fenkiou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a bit late ahah

Comment thread .github/workflows/preview.yml
Comment thread .github/workflows/publish-to-testpypi-and-pypi.yml
permissions:
contents: read

concurrency:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, once a new commit is pushed test/lint/type check can just be cancelled as only the latest one matters no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

As it is in this PR, in a PR I agree with you, when the commit sits in main or in a tag, I would leave it.

Reason behind would be if the test/lint/typecheck takes a bit longer, and we merge multiple PRs in a row, it can help pinpoint which commit broke the jobs.

We're nowhere near that kind of traffic in this repo, so I'm open to always have cancel-in-progress: true instead of cancel-in-progress: PR-only, WDYT?

@Fenkiou Fenkiou Sep 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can help pinpoint which commit broke the jobs

ok valid point even though I'm not sure it can happen as those jobs must pass before the PR lands in main, anyway keep it as is that's not an issue :)

Comment thread .github/workflows/verify-if-secrets-were-leaked.yml
Comment thread .github/workflows/publish-to-testpypi-and-pypi.yml
@sarjevane

sarjevane commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

a bit late ahah

@Fenkiou Apologies, did not know you were reviewing! Let me address your comments, and I can open a follow-up PR with whatever we agree to fix

@sarjevane sarjevane left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Created #297 as a follow-up.

Some questions are still open here, I can include them in #297 if you want me to:

  • remove persist-credentials altogether
  • cancel-in-progress for non-pr workflows

permissions:
contents: read

concurrency:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

As it is in this PR, in a PR I agree with you, when the commit sits in main or in a tag, I would leave it.

Reason behind would be if the test/lint/typecheck takes a bit longer, and we merge multiple PRs in a row, it can help pinpoint which commit broke the jobs.

We're nowhere near that kind of traffic in this repo, so I'm open to always have cancel-in-progress: true instead of cancel-in-progress: PR-only, WDYT?

Comment thread .github/workflows/publish-to-testpypi-and-pypi.yml
Comment thread .github/workflows/preview.yml
doc-publish-
- run: pip install .[docs]
working-directory: pasqal-cloud
- run: mkdocs gh-deploy --force

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Note: This is actually the only place that under the hood uses git, so that's the only place where we need the persist-credentials: true (the default value).

Comment thread .github/workflows/publish-to-testpypi-and-pypi.yml
push:
branches:
- main
- dev

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

dev branch isn't used anymore ;)


concurrency:
group: tests-${{ github.event.pull_request.number || github.ref }}
# Superseded pull request runs are pointless; runs on main/dev are not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

main/dev -> main ;)

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