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

Commit

Permalink
135: Support autoformatting and linting with Therapist (#142)
Browse files Browse the repository at this point in the history
This changeset contains a number of squashed 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).

**Please view the README and start using `therapist` to avoid other developers' commits containing formatting changes to parts of files they didn't explicitly work on**

Closes #135 

## Summarised changes: 
* full-codebase passes made with `black`, `isort`, `flake8`, `eslint` and `prettier` to bring everything into line
* add [`therapist`](https://github.com/rehandalal/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


## Squashed commits:

* 135: Full pass with `black`as first step towards autoformatting

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.

* 135: add isort with black-compatible config

Created a separate .isort.cfg to make it explicit

Tests passing fine after changes

* 135: tighten up flake8 to also enforce 88-char lines, matching black

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)

* 135: Run prettier across all SCSS, ahead of hooking it into therapist's pre-commit tasks

* 135: Tune prettier and eslint's ignore options

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.

* 135: Add Therapist to the project, running the linters and formatters 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

* 135: Update Readme to mention how to set up and use `therapist` as a linter/formatter runner
  • Loading branch information
Steve Jalim authored Sep 18, 2019
1 parent a19a976 commit bcf5684
Show file tree
Hide file tree
Showing 280 changed files with 7,255 additions and 3,637 deletions.
4 changes: 4 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
dist
*.scss
*.py
*.pyc
*.html
*.svg
13 changes: 13 additions & 0 deletions .isort.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[settings]
known_first_party=developerportal
known_third_party=wagtail,modelcluster,six,requests,willow,pillow,taggit
known_django=django
default_section=FIRSTPARTY
skip=migrations
sections=FUTURE,STDLIB,DJANGO,THIRDPARTY,FIRSTPARTY,LOCALFOLDER
multi_line_output=3
include_trailing_comma=True
force_grid_wrap=0
use_parentheses=True
line_length=88
skip=node_modules,migrations
5 changes: 5 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
dist
*.py
*.pyc
*.html
*.svg
*.json
42 changes: 42 additions & 0 deletions .therapist.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
actions:
# Python linting and formatting
isort:
run: isort --check {files}
fix: isort --apply {files}
include: '*.py'
exclude:
- 'node_modules'
- 'migrations'
flake8:
run: flake8 {files}
include: '*.py'
exclude:
- 'node_modules'
- 'migrations'
black:
run: black {files}
include: '*.py'
exclude:
- 'node_modules'
- 'migrations'
- '*.pyc'

# JS linting
eslint:
description: ESLint
run: $(npm bin)/eslint {files}
fix: $(npm bin)/eslint --fix {files}
exclude:
- '*.html'
- '*.md'
- '*.py'
- '*.pyc'
- '*.scss'
- '*.svg'
- 'developerportal/apps/common/fixtures/common.json'

# JS and SCSS autoformatting
prettier:
description: Prettier
run: $(npm bin)/prettier -c {files}
fix: $(npm bin)/prettier --write {files}
49 changes: 47 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

This project uses [Docker](https://www.docker.com/). The easiest way to get started is to install [Docker Desktop](https://hub.docker.com/search?q=Docker%20Desktop&type=edition&offering=community) which provides the `docker` and `docker-compose` commands.

After installing Docker, use the __dev-setup__ script to run the project locally:
After installing Docker, use the **dev-setup** script to run the project locally:

```shell
./scripts/dev-setup
Expand Down Expand Up @@ -36,9 +36,54 @@ docker-compose exec app python manage.py makemigrations
docker-compose exec app python manage.py migrate
```

### Linting and autoformatting with (or without) a pre-commit hook

The project has support for using [Therapist](https://github.com/rehandalal/therapist) to run pre-commit linting and formatting tools. You don't _have_ to use it, but it makes it life easier.

`therapist` is configured to run:

- [black](https://github.com/psf/black) for code formatting
- [flake8](http://flake8.pycqa.org/en/latest/) for syntax checking
- [isort](https://github.com/timothycrosley/isort/) for import-order management
- [eslint](https://eslint.org/) for JavaScript checking
- [prettier](https://prettier.io/) for JavaScript formatting

At the moment, this project assumes have all of the above installed and available on the `$PATH` of your _host_ machine, along with Python 3 and `pip`. You can install the dependencies using `virtualenv` and `nvm` if you want, or not, or globally. As long as they're available, you're good.

(Note: this project is not currently supporting use of `therapist` inside Docker containers.)

**TIP: It's wise to enable all of the above tooling in your code editor, if possible, so that things are already in order before the pre-commit hook is run.**

#### `therapist` setup and usage

Install `therapist` :

$ pip install therapist

Install the pre-commit hook that will trigger `therapist` automatically:

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

(Take a look at the pre-commit file added to `.git/hooks/` to confirm the path looks right.)

Now, when you `git-commit` a change, the _staged_ changes will be checked by one or more of `black`, `isort`, `flake8`, `eslint` and/or `prettier`. See `.therapist.yml` in the project root for the configuration.

Alternatively, if you wanted to run it across the whole codebase, run:

$ therapist run developerportal/

And if, for some reason, you want `therapist` to auto-fix everything wrong using those tools, run:

$ therapist run developerportal/ --fix

Finally, `therapist` can be passed a list of file paths if you want to just run it on specific things:

$ therapist run developerportal/path/to/file.js developerportal/path/to/another_file.py

### User authentication

GitHub OAuth is supported for admin login. When running in production, the auth pipeline checks to see if the GitHub user is a member of the __mdn__ organization. Additional organizations can be added via the `GITHUB_ORGS` env variable.
GitHub OAuth is supported for admin login. When running in production, the auth pipeline checks to see if the GitHub user is a member of the **mdn** organization. Additional organizations can be added via the `GITHUB_ORGS` env variable.

When running in debug any GitHub user is allowed to log in and is automatically given superuser status. It is possible to log in without using GitHub by creating a Django superuser as normal:

Expand Down
24 changes: 16 additions & 8 deletions developerportal/apps/articles/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,26 @@ class Migration(migrations.Migration):
initial = True

dependencies = [
('wagtailcore', '0041_group_collection_permissions_verbose_name_plural'),
("wagtailcore", "0041_group_collection_permissions_verbose_name_plural")
]

operations = [
migrations.CreateModel(
name='Article',
name="Article",
fields=[
('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')),
(
"page_ptr",
models.OneToOneField(
auto_created=True,
on_delete=django.db.models.deletion.CASCADE,
parent_link=True,
primary_key=True,
serialize=False,
to="wagtailcore.Page",
),
)
],
options={
'abstract': False,
},
bases=('wagtailcore.page',),
),
options={"abstract": False},
bases=("wagtailcore.page",),
)
]
18 changes: 11 additions & 7 deletions developerportal/apps/articles/migrations/0002_article_body.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@

class Migration(migrations.Migration):

dependencies = [
('articles', '0001_initial'),
]
dependencies = [("articles", "0001_initial")]

operations = [
migrations.AddField(
model_name='article',
name='body',
field=wagtail.core.fields.StreamField([('paragraph', wagtail.core.blocks.RichTextBlock()), ('image', wagtail.images.blocks.ImageChooserBlock())], default=None),
),
model_name="article",
name="body",
field=wagtail.core.fields.StreamField(
[
("paragraph", wagtail.core.blocks.RichTextBlock()),
("image", wagtail.images.blocks.ImageChooserBlock()),
],
default=None,
),
)
]
14 changes: 7 additions & 7 deletions developerportal/apps/articles/migrations/0002_article_date.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@

class Migration(migrations.Migration):

dependencies = [
('articles', '0001_initial'),
]
dependencies = [("articles", "0001_initial")]

operations = [
migrations.AddField(
model_name='article',
name='date',
field=models.DateField(default=datetime.date.today, verbose_name='Article date'),
),
model_name="article",
name="date",
field=models.DateField(
default=datetime.date.today, verbose_name="Article date"
),
)
]
12 changes: 5 additions & 7 deletions developerportal/apps/articles/migrations/0003_article_intro.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@

class Migration(migrations.Migration):

dependencies = [
('articles', '0002_article_date'),
]
dependencies = [("articles", "0002_article_date")]

operations = [
migrations.AddField(
model_name='article',
name='intro',
field=wagtail.core.fields.RichTextField(default=None, verbose_name='Intro'),
),
model_name="article",
name="intro",
field=wagtail.core.fields.RichTextField(default=None, verbose_name="Intro"),
)
]
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,32 @@

class Migration(migrations.Migration):

dependencies = [
('articles', '0002_article_body'),
]
dependencies = [("articles", "0002_article_body")]

operations = [
migrations.AlterField(
model_name='article',
name='body',
field=developerportal.apps.common.fields.CustomStreamField([('paragraph', wagtail.core.blocks.RichTextBlock(features=('bold', 'italic', 'link', 'ol', 'ul', 'blockquote', 'code', 'hr'))), ('image', wagtail.images.blocks.ImageChooserBlock())], default=None),
),
model_name="article",
name="body",
field=developerportal.apps.common.fields.CustomStreamField(
[
(
"paragraph",
wagtail.core.blocks.RichTextBlock(
features=(
"bold",
"italic",
"link",
"ol",
"ul",
"blockquote",
"code",
"hr",
)
),
),
("image", wagtail.images.blocks.ImageChooserBlock()),
],
default=None,
),
)
]
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@

class Migration(migrations.Migration):

dependencies = [
('articles', '0003_article_intro'),
]
dependencies = [("articles", "0003_article_intro")]

operations = [
migrations.AlterField(
model_name='article',
name='intro',
field=wagtail.core.fields.RichTextField(default='', verbose_name='Intro'),
),
model_name="article",
name="intro",
field=wagtail.core.fields.RichTextField(default="", verbose_name="Intro"),
)
]
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,9 @@

class Migration(migrations.Migration):

dependencies = [
('articles', '0004_auto_20190516_0907'),
]
dependencies = [("articles", "0004_auto_20190516_0907")]

operations = [
migrations.RemoveField(
model_name='article',
name='date',
),
migrations.RemoveField(
model_name='article',
name='intro',
),
migrations.RemoveField(model_name="article", name="date"),
migrations.RemoveField(model_name="article", name="intro"),
]
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@

class Migration(migrations.Migration):

dependencies = [
('articles', '0005_auto_20190516_1236'),
]
dependencies = [("articles", "0005_auto_20190516_1236")]

operations = [
migrations.AddField(
model_name='article',
name='date',
field=models.DateField(default=datetime.date.today, verbose_name='Article date'),
model_name="article",
name="date",
field=models.DateField(
default=datetime.date.today, verbose_name="Article date"
),
),
migrations.AddField(
model_name='article',
name='intro',
field=wagtail.core.fields.RichTextField(default='', verbose_name='Intro'),
model_name="article",
name="intro",
field=wagtail.core.fields.RichTextField(default="", verbose_name="Intro"),
),
]
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
class Migration(migrations.Migration):

dependencies = [
('articles', '0006_auto_20190516_1303'),
('articles', '0003_auto_20190517_1038'),
("articles", "0006_auto_20190516_1303"),
("articles", "0003_auto_20190517_1038"),
]

operations = [
]
operations = []
Loading

0 comments on commit bcf5684

Please sign in to comment.