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

Separate lint into several jobs #2467

Merged
merged 69 commits into from
Apr 30, 2024
Merged

Conversation

ocelotl
Copy link
Contributor

@ocelotl ocelotl commented Apr 26, 2024

Fixes #2419

@ocelotl ocelotl added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 26, 2024
@ocelotl ocelotl self-assigned this Apr 26, 2024
@ocelotl ocelotl requested a review from a team April 26, 2024 18:48
@ocelotl ocelotl marked this pull request as draft April 26, 2024 18:48
Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

FYI black, isort and flake8 can be run once per repository instead of per package, that would avoid duplicating a ton of lines in tox (and also work)

@ocelotl ocelotl force-pushed the issue_2419 branch 3 times, most recently from 1f65939 to 1638399 Compare April 26, 2024 21:57
@ocelotl
Copy link
Contributor Author

ocelotl commented Apr 26, 2024

FYI black, isort and flake8 can be run once per repository instead of per package, that would avoid duplicating a ton of lines in tox (and also work)

Sure, but I want to produce a scenario as similar as possible to a scenario where there are multiple tox.ini files, one per package.

@ocelotl ocelotl marked this pull request as ready for review April 26, 2024 22:45
tox.ini Show resolved Hide resolved
@ocelotl ocelotl force-pushed the issue_2419 branch 2 times, most recently from a4ea75f to b0e6efc Compare April 27, 2024 22:26
@ocelotl
Copy link
Contributor Author

ocelotl commented Apr 27, 2024

Lint tests take ~14 minutes right now: https://github.com/open-telemetry/opentelemetry-python-contrib/actions/runs/8839570393/job/24273211949

This PR speeds up this process to ~2 minutes: https://github.com/open-telemetry/opentelemetry-python-contrib/actions/runs/8862990640

@xrmx
Copy link
Contributor

xrmx commented Apr 29, 2024

FYI black, isort and flake8 can be run once per repository instead of per package, that would avoid duplicating a ton of lines in tox (and also work)

Sure, but I want to produce a scenario as similar as possible to a scenario where there are multiple tox.ini files, one per package.

I see that, I would had hoped to make lint faster by using faster tools before splitting stuff though.

@ocelotl
Copy link
Contributor Author

ocelotl commented Apr 29, 2024

FYI black, isort and flake8 can be run once per repository instead of per package, that would avoid duplicating a ton of lines in tox (and also work)

Sure, but I want to produce a scenario as similar as possible to a scenario where there are multiple tox.ini files, one per package.

I see that, I would had hoped to make lint faster by using faster tools before splitting stuff though.

Sure, we can move to another tool later, this does not introduce anyting new that needs further CI testing and already makes a huge difference.

@ocelotl ocelotl requested a review from xrmx April 29, 2024 22:40
Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

Would be nice to sort packages in the matrix though

@ocelotl
Copy link
Contributor Author

ocelotl commented Apr 30, 2024

Would be nice to sort packages in the matrix though

Right, I just sorted them ✌️

@ocelotl ocelotl merged commit 5116305 into open-telemetry:main Apr 30, 2024
314 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint is very slow
2 participants