Optimize workflow run by Cjkjvfnby · Pull Request #8225 · astral-sh/ruff

Cjkjvfnby · 2023-10-25T19:39:42Z

Summary

Don't run jobs that are not affected
Replace scripts/* with scripts/**

Test Plan

This PR changes ci.yaml, so it will trigger all jobs. there is no good way to test if something is skipped before merging this to the main and creating other PRs.

I am not familiar with the project well, so might miss some configs that are important.

.github/workflows/ci.yaml

MichaReiser

Thanks for improving our pipeline.

I like the idea of running fewer jobs but I'm unsure if the added complexity is worth it, considering that most jobs include code changes. It's also very easy to forget files.

MichaReiser · 2023-10-26T00:04:39Z

.github/workflows/ci.yaml

+              - Cargo.toml
+              - Cargo.lock
+              - clippy.toml
+              - pyproject.toml


Suggested change

- pyproject.toml

- pyproject.toml

- rust-toolchain.toml

- fuzz/**

Does the plugin support a way to say: Only run when e.g. all changed files are markdown files? It would reduce that we missed adding an entry. I rather have the job runs too often that miss out.

As I see no, it could not.

But it could be addressed by running them on master, after the merge.

Why not with something like

- **/* - !**/*.md

I would be surprised if this was not feasible.

.github/workflows/ci.yaml

Cjkjvfnby · 2023-10-29T13:17:59Z

Force pushed changes, after rebasing on top of the main

zanieb

I don't think we want to maintain this list of files. I'd prefer the "negated" style that @MichaReiser requested as well.

Now negeatin schema is used (all files except specified)

github-actions · 2023-11-04T13:45:36Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

Cjkjvfnby · 2023-11-04T13:48:35Z

The first implementation with negation.

code and formatter sections are the same, probably some exclusions should be added to formatted. I don't have expertise in the project to do it.

MichaReiser

The formatter now runs on all linter changes because it doesn't exclude the linter specific crates (or development-only crates).

I'm okay merging this but I would prefer reverting the changes to the linter/formatter filesets to keep the changes isolated.

Cjkjvfnby · 2023-11-29T20:38:39Z

The formatter now runs on all linter changes because it doesn't exclude the linter specific crates (or development-only crates).

I don't know Rust, so could not do it myself.

I'm okay merging this but I would prefer reverting the changes to the linter/formatter filesets to keep the changes isolated.

Done.

codspeed-hq · 2023-11-29T20:51:32Z

CodSpeed Performance Report

Merging #8225 will improve performances by 11.42%

_{Comparing Cjkjvfnby:sa/optimize-workflow-run (8ad224b) with main (5d554ed)}

Summary

⚡ 5 improvements
✅ 20 untouched benchmarks

🆕 5 new benchmarks

Benchmarks breakdown

	Benchmark	`main`	`Cjkjvfnby:sa/optimize-workflow-run`	Change
⚡	`linter/all-rules[unicode/pypinyin.py]`	17.2 ms	16.2 ms	+6.03%
⚡	`linter/all-rules[numpy/globals.py]`	4.2 ms	3.9 ms	+6.42%
⚡	`linter/all-rules[pydantic/types.py]`	80.3 ms	72.2 ms	+11.18%
🆕	`linter/all-with-preview-rules[unicode/pypinyin.py]`	N/A	17.2 ms	N/A
🆕	`linter/all-with-preview-rules[numpy/globals.py]`	N/A	4.2 ms	N/A
🆕	`linter/all-with-preview-rules[large/dataset.py]`	N/A	185.6 ms	N/A
🆕	`linter/all-with-preview-rules[numpy/ctypeslib.py]`	N/A	37.1 ms	N/A
⚡	`linter/all-rules[numpy/ctypeslib.py]`	36.8 ms	33.9 ms	+8.58%
⚡	`linter/all-rules[large/dataset.py]`	186.2 ms	167.1 ms	+11.42%
🆕	`linter/all-with-preview-rules[pydantic/types.py]`	N/A	79.9 ms	N/A

These were literals instead of expressions... and were consequently not evaluated. Fixes bug from #8225

Replaces #9035 Fixes #8225 The issue appears to be that `*/**` was used instead of `**/*` which did not match _any_ changed file as desired

zanieb reviewed Oct 25, 2023

View reviewed changes

.github/workflows/ci.yaml Outdated Show resolved Hide resolved

MichaReiser reviewed Oct 26, 2023

View reviewed changes

Cjkjvfnby closed this Oct 29, 2023

Cjkjvfnby reopened this Oct 29, 2023

zanieb requested changes Nov 1, 2023

View reviewed changes

Cjkjvfnby added 2 commits November 4, 2023 13:40

Don't run ci workflow when it has no sense

0c9481c

Change a way how wokrflow checks skipping the unnecessary workflows

078e764

Now negeatin schema is used (all files except specified)

MichaReiser added internal An internal refactor or improvement ci Related to internal CI tooling and removed internal An internal refactor or improvement labels Nov 27, 2023

MichaReiser requested changes Nov 27, 2023

View reviewed changes

Cjkjvfnby added 2 commits November 29, 2023 20:59

Merge branch 'main' into sa/optimize-workflow-run

0714d48

Revert changes in files_yaml

4e031df

Fix missed quotes

8ad224b

MichaReiser approved these changes Nov 30, 2023

View reviewed changes

MichaReiser merged commit 08f3110 into astral-sh:main Nov 30, 2023

This was referenced Dec 7, 2023

release: switch to Cargo's default #9031

Merged

Fix detection of changed files in CI #9035

Merged

zanieb added a commit that referenced this pull request Dec 7, 2023

Fix detection of changed files in CI (#9035)

06c9f62

These were literals instead of expressions... and were consequently not evaluated. Fixes bug from #8225

zanieb mentioned this pull request Dec 7, 2023

Fix determine changes detection of "code" changes #9038

Merged

zanieb added a commit that referenced this pull request Dec 7, 2023

Fix determine changes detection of "code" changes (#9038)

d22ce53

Replaces #9035 Fixes #8225 The issue appears to be that `*/**` was used instead of `**/*` which did not match _any_ changed file as desired

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Optimize workflow run#8225

Optimize workflow run#8225
MichaReiser merged 5 commits intoastral-sh:mainfrom
Cjkjvfnby:sa/optimize-workflow-run

Cjkjvfnby commented Oct 25, 2023 •

edited

Loading

Uh oh!

Uh oh!

MichaReiser left a comment

Uh oh!

MichaReiser Oct 26, 2023

Uh oh!

Cjkjvfnby Oct 29, 2023

Uh oh!

zanieb Nov 1, 2023

Uh oh!

Uh oh!

Cjkjvfnby commented Oct 29, 2023

Uh oh!

zanieb left a comment

Uh oh!

github-actions bot commented Nov 4, 2023 •

edited

Loading

Uh oh!

Cjkjvfnby commented Nov 4, 2023

Uh oh!

MichaReiser left a comment

Uh oh!

Cjkjvfnby commented Nov 29, 2023

Uh oh!

codspeed-hq bot commented Nov 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

Cjkjvfnby commented Oct 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

MichaReiser Oct 26, 2023

Choose a reason for hiding this comment

Uh oh!

Cjkjvfnby Oct 29, 2023

Choose a reason for hiding this comment

Uh oh!

zanieb Nov 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Cjkjvfnby commented Oct 29, 2023

Uh oh!

zanieb left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

Cjkjvfnby commented Nov 4, 2023

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Cjkjvfnby commented Nov 29, 2023

Uh oh!

codspeed-hq bot commented Nov 29, 2023

CodSpeed Performance Report

Merging #8225 will improve performances by 11.42%

Summary

Benchmarks breakdown

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Cjkjvfnby commented Oct 25, 2023 •

edited

Loading

github-actions bot commented Nov 4, 2023 •

edited

Loading

`ruff-ecosystem` results