Skip to content
This repository has been archived by the owner on Dec 9, 2021. It is now read-only.

135: Support autoformatting and linting with Therapist #142

Merged
merged 7 commits into from
Sep 18, 2019

Conversation

stevejalim
Copy link
Contributor

This changeset contains a number of commits towards standardising the formatting and linting of the devportal project. (It wasn't bad before, just good to get this in during a quietish patch of development).

Closes #135

Summarised changes:

  • full-codebase passes made with black, isort, flake8, eslint and prettier to bring everything into line
  • add therapist as a framework for running the above tooling (either via a pre-commit hook or directly - both paths are available)

+7,255 -3,637 looks huge, but the vast majority of the diff is standardising formatting

To test (if you want to):

  • pull this branch
  • review the new section in the README about installing and using therapist
  • do what the README says :o)
  • confirm a full sweep works and detects no problems
  • try committing a minor change that breaks formatting (eg: add some returns in a Python file and save it without a code-editor's black plugin fixing it) - confirm that therapist's pre-commit hook will block the commit

Steve Jalim added 7 commits September 17, 2019 10:14
252 files reformatted, 42 files left unchanged.

Done with `black --exclude node_modules .` to avoid unnecessarily reformatting third-party package helper Python in JS deps

Includes migration files for consistency, even though it adds noise (209/252 files are migrations...)

Tests still passing.
Created a separate .isort.cfg to make it explicit

Tests passing fine after changes
This commit contains some (manual) changes to over-long lines caught by flake8 that black _didn't_ auto-fix, because they were long strings (see psf/black#182)
Specifically:
- prettier: ignore HTML, SVG, JSON and Python
- eslint: ignore HTML, SVG and Python

While this wouldn't be needed for editor-based formatting or linting, nor linting explicitly staged/passed files, this change is so that when we _do_ run them across the entire `developerportal` module (eg, in CI), they won't gripe about things they are not intented to be looking at anyway.
… recently added/tweaked

At this point, the assumption is you have the following installed on your host:
* black
* flake8
* eslint and eslint-config-prettier
* isort
* prettier
(I'll deal with making that easier in a different commit.)

Install `therapist` (https://github.com/rehandalal/therapist):

	$ pip install therapist

Now install the pre-commit hook that will trigger Therapist automatically:

	$ therapist install
	Installing pre-commit hook...	DONE

Now, when you commit a change, the staged changes will be checked by one or more of black, isort, eslint and prettier, as appropriate. See `.therapist.yml` for the configuration.

Alternatively, if you wanted to run it across the whole codebase (as we might well do in CI), run:

	$ therapist run developerportal/
	black ............................................................... [SUCCESS]
	ESLint .............................................................. [SKIPPED]
	flake8 .............................................................. [SUCCESS]
	isort ............................................................... [SUCCESS]
	Prettier ............................................................ [SUCCESS]

-------------------------------------------------------------------------------
Completed in: 3.67s

And if you want therapist to auto-fix things using black, prettier and/or eslint, run:

	$ therapist run developerportal --fix
@stevejalim stevejalim self-assigned this Sep 17, 2019
@stevejalim stevejalim added the housekeeping Non-functional, but necessary, changes label Sep 17, 2019
Copy link
Contributor

@peterbe peterbe left a comment

Choose a reason for hiding this comment

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

I skimmed it quickly and I think it's probably fine. It's clear that you've understood the power of letting computers do the boring work for humans.
I didn't really scrutinize the details but it looks like you know what you're talking about.
I might attempt to copy some of these over to Kuma once that project upgrades to Python3.

@stevejalim
Copy link
Contributor Author

@dcgauld Heya David - we haven't met yet, but I'm helping out on the project from the Mozilla end. I'm mindful that I don't want to cause pain for you/Potato, so wondered if you have any objection to me merging this PR to master?

@dcgauld
Copy link
Contributor

dcgauld commented Sep 18, 2019

@stevejalim Hey Steve, awesome! Looking forward to chatting to you properly soon. No objections at all, it looks great and was pretty overdue – thank you!

@stevejalim stevejalim merged commit bcf5684 into master Sep 18, 2019
@stevejalim stevejalim deleted the 135-autoformat-and-lint-with-therapist branch September 18, 2019 11:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
housekeeping Non-functional, but necessary, changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add auto-formatting and linting
3 participants