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: set concurrency #112

Merged
merged 10 commits into from
Mar 23, 2024
Merged

CI: set concurrency #112

merged 10 commits into from
Mar 23, 2024

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Mar 8, 2024

Found this use-case for GH action and Tox - https://github.com/tox-dev/tox-gh; what do you think?

@Borda Borda marked this pull request as ready for review March 8, 2024 21:13
@Borda Borda mentioned this pull request Mar 8, 2024
@hugovk
Copy link
Collaborator

hugovk commented Mar 8, 2024

I don't really understand what tox-gh brings, beyond duplicating the Python version numbers in test.yml and in tox.ini

Please could you elaborate on the benefits?

@Borda
Copy link
Contributor Author

Borda commented Mar 8, 2024

Please could you elaborate on the benefits?

You are probably right, just found it easier to read/follow...

@Borda Borda marked this pull request as draft March 8, 2024 21:31
@JulienPalard
Copy link
Collaborator

I'm not a fan of bringing yet another dependency here. If it was me alone I would just have a single github build (with multiple actions/setup-python in it, yes it works) running a single tox instance: trying to keep it simple, a bit like in 45af71c.

@Borda
Copy link
Contributor Author

Borda commented Mar 9, 2024

it was me alone I would just have a single github build (with multiple actions/setup-python in it, yes it works) running a single tox instance: trying to keep it simple

Yes, agree but since the Tox is here, it is better to check it and the simplest was to do so is use it... So to say reduce dead code =)

a bit like in 45af71c.

Well, this one is also strange with redundant python setups...

Copy link
Collaborator

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Anyway, back to this PR.

I think the concurrency: changes are useful, and let's revert the others.

.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
@Borda
Copy link
Contributor Author

Borda commented Mar 9, 2024

Anyway, back to this PR.

OK will update PR and it's name accordingly

Borda and others added 2 commits March 9, 2024 11:14
@Borda Borda changed the title ci: refactor Tox in action with tox-gh ci: setting concurrency Mar 9, 2024
@Borda Borda marked this pull request as ready for review March 9, 2024 15:03
@Borda Borda requested a review from hugovk March 9, 2024 15:03
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/lint.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <[email protected]>
@Borda Borda requested a review from hugovk March 11, 2024 13:11
Co-authored-by: Hugo van Kemenade <[email protected]>
Copy link
Collaborator

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks!

@hugovk hugovk changed the title ci: setting concurrency CI: set concurrency Mar 11, 2024
@Borda
Copy link
Contributor Author

Borda commented Mar 14, 2024

@hugovk so fine to land? 🐿️

@Borda
Copy link
Contributor Author

Borda commented Mar 19, 2024

@hugovk is it fine now, or shall I update something else? 🦩

@hugovk hugovk merged commit a9559ba into sphinx-contrib:main Mar 23, 2024
21 checks passed
@Borda Borda deleted the ci/tox branch March 23, 2024 13:47
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.

3 participants