Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

copy workflows from blueapi, [still need to configure env values -not a code change] #664

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

stan-dot
Copy link
Contributor

@

@stan-dot stan-dot added enhancement New feature or request github_actions Pull requests that update GitHub Actions code labels Nov 21, 2024
@stan-dot stan-dot requested a review from coretl November 21, 2024 16:51
@stan-dot stan-dot self-assigned this Nov 21, 2024
@stan-dot stan-dot linked an issue Nov 21, 2024 that may be closed by this pull request
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@stan-dot stan-dot force-pushed the 662-add-codeql-and-sonarcloud-steps-to-the-ci branch from 8562d66 to 5f9ea2e Compare November 25, 2024 15:36
@stan-dot stan-dot marked this pull request as ready for review December 6, 2024 15:01
Copy link
Collaborator

@coretl coretl left a comment

Choose a reason for hiding this comment

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

Please can you:

  • compress codeql.yaml down so that it only contains the python bits
  • turn it into a reusable workflow like _tox.yaml and call it from periodic.yaml
  • delete sonarcloud things

@stan-dot stan-dot force-pushed the 662-add-codeql-and-sonarcloud-steps-to-the-ci branch from 959cad1 to cfe48a3 Compare December 11, 2024 10:45
@stan-dot
Copy link
Contributor Author

thanks for the comments @coretl . I deleted the 'if swift language' branches, kept the comments though to keep this more similar to the template workflow for codeql. now sure fully about the syntax in the periodic file though

sonar-project.properties Outdated Show resolved Hide resolved
.github/workflows/_codeql.yaml Outdated Show resolved Hide resolved
.github/workflows/_codeql.yaml Outdated Show resolved Hide resolved
.github/workflows/_codeql.yaml Outdated Show resolved Hide resolved
.github/workflows/_codeql.yaml Outdated Show resolved Hide resolved
@stan-dot stan-dot force-pushed the 662-add-codeql-and-sonarcloud-steps-to-the-ci branch from 2a1df5b to ff9f5e4 Compare January 10, 2025 15:00
@stan-dot
Copy link
Contributor Author

deleted outdated comments

@stan-dot stan-dot requested a review from coretl January 15, 2025 12:09
Comment on lines 24 to 29
strategy:
fail-fast: false
matrix:
include:
- language: python
build-mode: none
Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant code

Suggested change
strategy:
fail-fast: false
matrix:
include:
- language: python
build-mode: none

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why is it redundant? the default ci.yml did use the reference to that

Copy link
Collaborator

Choose a reason for hiding this comment

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

You make a matrix with precisely one entry in it, saying that all jobs in the matrix should complete if one fails, defining the variables language and build mode. You don't use those variables anywhere. If you delete this block of code then the workflow will do exactly the same.

.github/workflows/_codeql.yaml Outdated Show resolved Hide resolved
Comment on lines 13 to 22
permissions:
# required for all workflows
security-events: write

# required to fetch internal or private CodeQL packs
packages: read

# only required for workflows in private repositories
actions: read
contents: read
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect these permissions need to be in periodic.yml rather than here. Please add them one at a time and test using https://github.com/bluesky/ophyd-async/actions/workflows/periodic.yml until it is working.

@stan-dot stan-dot force-pushed the 662-add-codeql-and-sonarcloud-steps-to-the-ci branch from ff9f5e4 to 5bcd8a8 Compare January 16, 2025 09:37
@stan-dot stan-dot force-pushed the 662-add-codeql-and-sonarcloud-steps-to-the-ci branch from 6d4a8ec to 999ec8c Compare January 20, 2025 17:13
@stan-dot
Copy link
Contributor Author

@coretl I think all is clear now

@stan-dot stan-dot requested a review from coretl January 20, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request github_actions Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CodeQL and Sonarcloud steps to the CI
2 participants