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

Set up pre-commit hooks and workflows to check code style #313

Merged
merged 5 commits into from
Mar 28, 2024

Conversation

aws-tianquaw
Copy link
Contributor

Issue #, if available:
Currently the code base doesn't have any code stile check machnism.

Description of changes:

  • Add pre-commit hooks to run code style formatters including black, isort, autoflake
  • Run formatters on all existing files in this code base
  • Add a git action workflow to check code styles whenever a new pull request is created and whenever something is pushed

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@claytonparnell claytonparnell left a comment

Choose a reason for hiding this comment

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

We should run all the unit tests (tests/test_main.py etc.) to make sure none of the functionality is impacted. And probably run a build/test/push as well.

DEVELOPMENT.md Outdated Show resolved Hide resolved
@balajisankar15
Copy link
Contributor

Once this is merged, can you also rebase 2.0.0-beta with these changes?

@aws-tianquaw
Copy link
Contributor Author

We should run all the unit tests (tests/test_main.py etc.) to make sure none of the functionality is impacted. And probably run a build/test/push as well.

Executed the local build/test and got the following results:

=========================== short test summary info ============================
FAILED tests/unit/sagemaker/serve/builder/test_transformers_builder.py::TestTransformersBuilder::test_build_deploy_for_trans
formers_local_container_and_remote_container
FAILED tests/unit/sagemaker/serve/model_server/triton/test_server.py::TritonServerTests::test_start_invoke_destroy_local_tri
ton_server_cpu
FAILED tests/unit/sagemaker/serve/model_server/triton/test_server.py::TritonServerTests::test_start_invoke_destroy_local_tri
ton_server_gpu
= 3 failed, 7668 passed, 3778 skipped, 2 deselected, 3 xpassed, 3308 warnings in 1619.18s (0:26:59)

The failed tests exactly match what is documented in #316, so we can conclude that this PR doesn't introduce any new test failure.

@aws-tianquaw
Copy link
Contributor Author

Once this is merged, can you also rebase 2.0.0-beta with these changes?

Sure I'll make sure to do it once this PR is merged

@aws-tianquaw aws-tianquaw merged commit 58e9920 into aws:main Mar 28, 2024
1 check passed
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