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

Add actions pipeline #214

Merged
merged 2 commits into from
Oct 1, 2024
Merged

Add actions pipeline #214

merged 2 commits into from
Oct 1, 2024

Conversation

sfowl
Copy link
Collaborator

@sfowl sfowl commented Sep 26, 2024

Adds basic Actions CI pipeline.

@sfowl sfowl self-assigned this Sep 26, 2024
@sfowl sfowl mentioned this pull request Sep 26, 2024
1 task
Copy link
Collaborator

@ccronca ccronca left a comment

Choose a reason for hiding this comment

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

lgtm

branches: ["development", "main"]

permissions:
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.

Do you know how the workflow will behave if any of the pre-commit tasks tries to make changes to the code, for example the reorder-python-imports ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would have modified the copy of the code at the linting step and then I believe tested the modified code in the subsequent step. I think this is undesirable so I've swapped the order of steps, code is tested first, then linted (which can end up in modified code).

Currently all the steps run sequentially, though some could be made to run in parallel I feel this is good enough for now. I will parallelize these steps when we move to a tekton based model.

I believe there's also work planned to use ruff for linting, which could replace many of the pre-commit linters and should be easier to make only check the code, not modify it. Til then this approach is low cost to centralize and automate the established tests/linters.

@sfowl sfowl merged commit 630a06d into development Oct 1, 2024
1 check passed
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.

2 participants