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.

Sign up for GitHub

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

Jump to bottom

Document code style and pre-commit hooks #1594

Merged
merged 2 commits into from
Jun 24, 2019
Merged

Document code style and pre-commit hooks #1594

merged 2 commits into from
Jun 24, 2019

Conversation

robertodr
Copy link
Contributor

@robertodr robertodr commented Apr 7, 2019 •
edited
Loading

Description

Breaks off documentation and .travis.yml linting off of #809. I have added a .pre-commit-config.yaml that will check for:

  • Filenames that would conflict on a case-insensitive filesystem
  • Files that contain merge conflict strings.
  • Python formatting with YAPF.
  • C++ formatting with clang-format.

Hooks are opt-in and require you to install pre-commit (with pip or conda) and then run pre-commit install to integrate the actual hooks.

Todos

Notable points (developer or user-interest) that this PR has or will accomplish.

  • Document code style and how to enforce it.
  • Add .pre-commit-config.yaml and explain how to use it.
  • Use build stages on Travis and lint/clean up .travis.yml

Status

  • Ready for review
  • Ready for merge

loriab
loriab reviewed Apr 8, 2019
View reviewed changes
.travis.yml Outdated Show resolved Hide resolved
doc/sphinxman/source/code_style.rst Outdated Show resolved Hide resolved
doc/sphinxman/source/code_style.rst Outdated Show resolved Hide resolved
fevangelista
fevangelista approved these changes Apr 19, 2019
View reviewed changes
Copy link
Contributor

@fevangelista fevangelista left a comment

Choose a reason for hiding this comment

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

This is something we really need, thanks for adding it!

dgasmith
dgasmith approved these changes Jun 24, 2019
View reviewed changes
Copy link
Member

@dgasmith dgasmith left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! Please rebase and we will merge it.

@robertodr
Copy link
Contributor Author

robertodr commented Jun 24, 2019 •
edited
Loading

Rebased! I had forgotten about this one...

@robertodr robertodr removed the request for review from amjames June 24, 2019 14:34
@loriab loriab added this to the Psi4 1.4 milestone Jun 24, 2019
@loriab loriab merged commit 6949cc4 into psi4:master Jun 24, 2019
@robertodr robertodr deleted the precommit branch June 29, 2019 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgasmith dgasmith dgasmith approved these changes

@loriab loriab loriab approved these changes

@fevangelista fevangelista fevangelista approved these changes

Assignees
No one assigned
Labels
documentation
Projects
None yet
Milestone
Psi4 1.4
Development

Successfully merging this pull request may close these issues.
4 participants
@robertodr @dgasmith @loriab @fevangelista