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

ci: split CI workflow and add ruff format #8

Merged
merged 7 commits into from
Dec 5, 2024

Conversation

yasuhito
Copy link
Contributor

@yasuhito yasuhito commented Dec 5, 2024

✍ Description

Split ci.yaml into three separate files for better organization:

  • mypy.yaml
  • ruff.yaml
  • test.yaml

Also added ruff format command in addition to ruff check in ruff.yaml.
This change makes it easier to manage each CI task and simplifies future maintenance.

- Split ci.yaml into mypy.yaml, ruff.yaml, and test.yaml
- Add ruff lint command to ruff.yaml
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/tranqu/transpiler_dispatcher.py 78.33% <100.00%> (+16.66%) ⬆️

@yasuhito yasuhito changed the title ci: split CI workflow and add ruff lint ci: split CI workflow and add ruff format Dec 5, 2024
Copy link
Contributor

@snuffkin snuffkin left a comment

Choose a reason for hiding this comment

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

Tranqu's ci flow is small, so it's harder to manage if the files are split up.
I think it's a good idea to add ruff format, so I'd like to keep the files as one.

@yasuhito
Copy link
Contributor Author

yasuhito commented Dec 5, 2024

I still believe there are several advantages to splitting the CI workflows:

Faster Execution through Parallelization

  • Each check (ruff, mypy, pytest) can run in parallel, reducing overall execution time
  • This becomes particularly beneficial as tests grow in number

Efficient Error Identification

  • When checks run independently, you can identify all issues at once if multiple problems exist
  • For example, even if ruff fails, you can still see errors from mypy and pytest simultaneously
  • This reduces the cycle of fix → push → CI error → fix...

Improved Maintainability

  • Managing each check's configuration in separate files makes the intent clearer
  • Change history becomes more transparent for each type of check
  • Makes it easier to add new checks or remove existing ones

Efficient Re-runs

  • If a specific check fails, you can re-run just that workflow
  • No need to run all checks every time, leading to more efficient use of CI resources

While the project may be small now, these advantages are significant when considering future scalability.

@snuffkin
Copy link
Contributor

snuffkin commented Dec 5, 2024

Tranqu is currently a small product, so if any problems arise in the future, we would like to consider splitting the files.
Splitting the files will increase duplicate codes. This will make it more difficult to manage.
Looking at the actual execution time for Tranqu's workflow, it is around 30s - 50s, so even if we run it in parallel now, the effect will be small.

@yasuhito
Copy link
Contributor Author

yasuhito commented Dec 5, 2024

Thank you for your detailed explanation. While I still see value in workflow separation for error identification and maintainability beyond just execution time, I understand your point about the current scale of Tranqu and the potential overhead from duplicate code.

Let's keep it as a single workflow file for now. We can always revisit this discussion when Tranqu grows, and the benefits of parallel execution become more significant.

I'll update the PR to consolidate the workflows into a single file while adding the ruff format check.

@yasuhito yasuhito requested a review from snuffkin December 5, 2024 02:23
Copy link
Contributor

@snuffkin snuffkin left a comment

Choose a reason for hiding this comment

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

LGTM!

@snuffkin snuffkin merged commit 3ffb695 into oqtopus-team:main Dec 5, 2024
4 of 5 checks passed
@yasuhito yasuhito deleted the feature/refactor-workflows branch December 5, 2024 02:52
@snuffkin snuffkin added the ci Adding or updating CI configuration and scripts label Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Adding or updating CI configuration and scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants