Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32165 closed New feature (fixed)

Add pre-commit Hooks

Reported by: David Smith Owned by: David Smith
Component: Packaging Version: dev
Severity: Normal Keywords:
Cc: Adam Johnson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

To add support for pre-commit hooks via pre-commit. Specifically, I propose to add the flake8 and isort hooks, with appropriate guidance added to the contributing docs.

The aim is to reduce the number of PRs opened where a 'simple' lint error needs to be fixed, and in doing so we end up running the whole test suite at Jenkins twice. I appreciate this won't stop all instances as folk will have an extra step to install them; however, I think even if it stops the occasional occurrence the addition will be worthwhile.

Change History (7)

comment:1 by Carlton Gibson, 3 years ago

Resolution: needsinfo
Status: newclosed

Hi David. I'd be inclined to accept this — I think pre-commit is great — but I think we need to go via the DevelopersMailingList first to gather consensus.

Ref DEP 8 this would be handy to have in place.

Can I get you to post to the list? Some thoughts...

  • Things we could set up now: the whitespace rule, flake8, isort.
  • Add black when DEP 8 is implemented.
  • What's the setup? Add to docs where?
  • ...

Thanks

comment:2 by Adam Johnson, 3 years ago

Cc: Adam Johnson added

comment:3 by Carlton Gibson, 3 years ago

Component: UncategorizedPackaging
Has patch: set
Resolution: needsinfo
Status: closednew
Triage Stage: UnreviewedReady for checkin
Type: UncategorizedNew feature

PR

(This is about GitHub process. I have no idea what component to assign it to really.)

comment:4 by Carlton Gibson, 3 years ago

Owner: changed from nobody to David Smith
Status: newassigned

comment:5 by Carlton Gibson <carlton@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 3bca95cc:

Fixed #32165 -- Added pre-commit hooks

  • Added pre-commit hooks for isort, flake8 and eslint
  • Added documentation on how to install and use the tool

comment:6 by Carlton Gibson, 3 years ago

In 3a0ed0ce:

Refs #31265 -- Updated .eslintignore to match eslint tests

(Moved comment due to wrong ticket number)

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In dc2ddfe9:

Refs #32165 -- Bumped minimum ESLint version to 7.16.0 to match pre-commit configuration.

Note: See TracTickets for help on using tickets.
Back to Top