Cookiecutter-django: Teach Travis to black Cookiecutter Django code

Created on 9 Apr 2018  路  19Comments  路  Source: pydanny/cookiecutter-django

black VCS integration.

x-ref #1602.

project infrastructure

Most helpful comment

By the time the code reaches Travis, it's already committed, so we'll need to create a new commit to reformat with black, posing the question of who should author it?

In my experience with pre-commit hooks, it's best to have them set up for each contributor. It looks feasible with https://pre-commit.com/#install, we _just_ need our contributors to have it installed and set up.

On the Travis side, maybe we can run black and fail the build if there is any change? We could show an error message letting the contributor know how to setup pre-commit?

If someone is more experienced with pre-commit, please correct me if I'm wrong!

All 19 comments

Did you have success with black's pre-commit integration?

I just tried with one of my projects and did not seem to work. See https://github.com/ambv/black/issues/117

@sfdye didn't have time to look into yet :(

@webyneter Sure, I thought you figured this out already 馃槤

The upstream issue seems to be fixed so I can start working on this. But I am not sure what you meant by let travis to black the code? @webyneter

@sfdye I meant, to automatically enforce black's code style in the repo prior to merging any commit to master.

@webyneter How to achieve that? please enlighten me 馃槃

If only I knew :)

By the time the code reaches Travis, it's already committed, so we'll need to create a new commit to reformat with black, posing the question of who should author it?

In my experience with pre-commit hooks, it's best to have them set up for each contributor. It looks feasible with https://pre-commit.com/#install, we _just_ need our contributors to have it installed and set up.

On the Travis side, maybe we can run black and fail the build if there is any change? We could show an error message letting the contributor know how to setup pre-commit?

If someone is more experienced with pre-commit, please correct me if I'm wrong!

On the Travis side, maybe we can run black and fail the build if there is any change? We could show an error message letting the contributor know how to setup pre-commit?

Sounds reasonable.

By the time the code reaches Travis, it's already committed

Obviously, should've thought about that before making the proposal :)

Another alternative as well: https://stickler-ci.com/docs#black

It makes comment on PRs and has a fixer option.

@browniebroke This looks promising!

If @pydanny and other maintainers have no objections, I could start a PR for a stickler-ci integration PoC?

Trying this out on my fork, I suspect it might not work due to the template tags in the python code...

Ah..I see

Might be worth it to submit an issue on black?

@browniebroke I suspect it was encountered before and it sure did:

https://github.com/pydanny/cookiecutter-django/pull/1602#issue-180182761

This means the whole file gets ignored because one line could not be parsed. I am not sure if this is acceptable if we were to black the whole project.

Yes, I remember this comment! I think there are 2 separate things we might want to do:

  1. Run black with --check on the generated project in test_docker.sh
  2. Run black on the utility scripts of the template itself

We can configure which paths should be considered by black, both at the template level and generated project level.

The template could check all files except {{cookiecutter.project_slug}} while the generated project could run on everything.

Thoughts?

Was this page helpful?
0 / 5 - 0 ratings