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

clean up some unnecessary files #3012

Merged
merged 3 commits into from Feb 21, 2020
Merged

Conversation

guihao-liang
Copy link
Collaborator

  • .flake8.log we don't need to track it in our repo.
  • black doesn't need to check py38.
  • ignore .pre-commit-config.yaml so that one can have their own customized pre-commit config living together with the repo.

@guihao-liang
Copy link
Collaborator Author

intentionally ignore,

  • E501, line too long
  • E266 too many leading '#' for block comment
  • temporarily ignore all init.py files
flake8 src/python/turicreate --config=tox.ini | wc -l
   864

Copy link
Collaborator

@TobyRoseman TobyRoseman left a comment

Choose a reason for hiding this comment

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

Do we want to add instructions for running flake8 to our build document?

Do we want to add flake8 to our development requirements.txt?

tox.ini Outdated
@@ -1,3 +1,4 @@
[flake8]
max-line-length = 88
extend-ignore = E501
extend-ignore = E501, E203, E501, E266
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need to list E501 twice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, my oversight.

.gitignore Outdated
@@ -105,3 +105,6 @@ deps/build
# CCache within Docker
/.docker_ccache/
/.docker_images/

# Pre-commit stuff
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really think this comment is really adding anything. I'd probably prefer just not having a comment here than having this comment. A better comment might be # git hooks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's better! Will do.

@guihao-liang
Copy link
Collaborator Author

Do we want to add instructions for running flake8 to our build document?

Do we want to add flake8 to our development requirements.txt?

No, this flake8 is only used by pre-commit hooks and also personal use. Currently, there are too many things that we need to fix in order to use flake8.

@guihao-liang guihao-liang merged commit 0b86e07 into apple:master Feb 21, 2020
@guihao-liang guihao-liang deleted the 02-19-20-clean branch February 21, 2020 17:12
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.

None yet

2 participants