Skip to content

Update sdist rules to include tox.toml (#3389) #3390

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

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Oct 3, 2024

Update sdist rules to include tox.toml, rather than tox.ini that no longer exists. This fixes test failures due to missing tox configuration.

Fixes #3389

  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

Sorry, something went wrong.

@mgorny mgorny requested a review from gaborbernat as a code owner October 3, 2024 14:47
Update sdist rules to include `tox.toml`, rather than `tox.ini` that
no longer exists.  This fixes test failures due to missing tox
configuration.

Fixes tox-dev#3389
@gaborbernat gaborbernat enabled auto-merge (squash) October 3, 2024 14:51
@gaborbernat gaborbernat merged commit 3ab1d3e into tox-dev:main Oct 3, 2024
28 checks passed
@mgorny mgorny deleted the tox-toml-sdist branch October 3, 2024 15:13
@webknjaz
Copy link
Contributor

webknjaz commented Oct 7, 2024

@gaborbernat would you mind if I send a PR making the CI test from sdist that would've caught this?

@gaborbernat
Copy link
Member

Neah, I prefer the faster behavior 99% over this rare edge case. And I do not think it would have caught it, because there still was the file locally.

@webknjaz
Copy link
Contributor

webknjaz commented Oct 7, 2024

@gaborbernat with my approach it would as I fully replace a Git checkout with untaring the sdist.

@gaborbernat
Copy link
Member

gaborbernat commented Oct 7, 2024

Neah. Not worth it, IMHO, prefer parity between local and CI. The 1% this happens, we can push out a fix.

@gaborbernat
Copy link
Member

https://github.com/henryiii/check-sdist I think actually would be a better approach to edit

@webknjaz
Copy link
Contributor

webknjaz commented Oct 8, 2024

@gaborbernat
Copy link
Member

I prefer Henry's solution as I find it less heavy-handed and works locally too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4.21.1 test regressions, probably resulting from tox.ini being removed from sdist
3 participants