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

Update docs for pre-commit (auto linting) #3332

Merged
merged 4 commits into from
Dec 1, 2017
Merged

Conversation

humitos
Copy link
Member

@humitos humitos commented Nov 29, 2017

No description provided.

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Noted some changes for clarity

Also, it would be good to put the standalone command usage here, as we're be running like this for now.

$ unify --quote="'"
$ yapf --exclude=*migrations* --exclude=*settings* --exclude=*scripts* --parallel
We have a strict code style that it's easy to follow since you just have to
install `pre-commit`_ and it will be ran automatically as a pre-commit hook for
Copy link
Contributor

Choose a reason for hiding this comment

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

ran -> run

Also, can drop the pre-commit hook for git, as we aren't actually prescribing that use yet.

$ yapf --exclude=*migrations* --exclude=*settings* --exclude=*scripts* --parallel
We have a strict code style that it's easy to follow since you just have to
install `pre-commit`_ and it will be ran automatically as a pre-commit hook for
git. This hook will run a different linting tools (`autoflake`_, `autopep8`_,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also hook here -- we should prescribe running this as a standalone command for now.

git. This hook will run a different linting tools (`autoflake`_, `autopep8`_,
`docformatter`_, `isort`_, `prospector`_, `unify`_ and `yapf`_) to check your
changes before you commit them and let you know if there are some problems that
weren't able to fix automatically.
Copy link
Contributor

Choose a reason for hiding this comment

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

weren't able to fix -> weren't fixed

commands::

$ pip install -U pre-commit
$ pre-commit install
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps note the pre-commit install step as a separate "beta" usage of precommit hooks

@agjohnson
Copy link
Contributor

@humitos Just got a chance to do the grammar fixes. I also cleared up our current usage of pre-commit. If curious, you can take a look in 2a20ae4

@agjohnson agjohnson merged commit 515f365 into master Dec 1, 2017
@agjohnson agjohnson deleted the humitos/docs/linting branch December 1, 2017 06:51
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