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

[EXP] Set up black and pre-commit hooks #680

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
1 participant
@luizirber
Copy link
Member

commented May 15, 2019

(To be fair, I don't see this getting merged =])

During the pycon sprints I used tox to set up the pre-commit checks (including formatting code with black). I really liked the experience because

  1. It simplifies setup. pip install tox and you're good to go, since tox will install all dependencies and build the package correctly.
  2. What is executed locally is what is executed in Travis. While we do have tox set up, I end up using make test (which install and runs tests outside of tox)
  3. Using pre-commit hooks avoid all the discussion about formatting code or other stylistic changes. And make it all consistent without needing intervention from the user (which was annoying with khmer, because it was up to the PR submitter to fix formatting issues).

A big change happening here is that the sourmash and sourmash_lib dirs were moved into src,
This is similar to a shadow branch I did some time ago, inspired by this discussion (more info).
The main benefit is avoiding confusing about what sourmash module gets imported for tests. The drawback is that Rust also uses the src dir for code, so now the Rust code and the Python code are mixed in the same dir... I would probably move the Rust code to another dir to avoid confusion.

Checklist

  • Is it mergeable?
  • make test Did it pass the tests?
  • make coverage Is the new code covered?
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Was a spellchecker run on the source code and documentation after
    changes were made?

@luizirber luizirber force-pushed the tox_black branch from b2f30e2 to d2356a5 May 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.