Skip to content
This repository has been archived by the owner on Jul 3, 2023. It is now read-only.

[ci] remove 'test_suite' in setup.py #257

Merged
merged 1 commit into from
Dec 24, 2022
Merged

[ci] remove 'test_suite' in setup.py #257

merged 1 commit into from
Dec 24, 2022

Conversation

jameslamb
Copy link
Contributor

Changes

  • removes unnecessary argument test_suite in setuptools.setup() in setup.py

How I tested this

docker run \
  --rm \
  --entrypoint="" \
  -v "$(pwd)":/opt/testing \
  --workdir /opt/testing \
  --env TASK=tests \
  -it circleci/python:3.9 \
  /bin/bash -c '.ci/setup.sh && .ci/test.sh'

Saw that test collection succeeded and all tests ran successfully.

===== 355 passed, 1 warning in 4.87s ====

Notes

This project uses pytest directly, and doesn't need any of the configuration for python setup.py test.

This change reduces the risk of hamilton packaging and installation failing on future versions of setuptools which remove python setup.py test support completely.

See relate PR #89 for links to documentation on ongoing changes to setuptools.

Thanks for your time and consideration.

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

@skrawcz skrawcz self-requested a review December 24, 2022 06:28
Copy link
Collaborator

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @jameslamb :shipit:

@skrawcz skrawcz merged commit 4f3e2e2 into stitchfix:main Dec 24, 2022
@jameslamb jameslamb deleted the setup-cleanup branch December 24, 2022 06:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants