Add a basic pre-commit lint and autoformat

[adehad]

Following up on: #438

Summary of changes

1. a pre-commit config file has been added with some simple rules
2. Errors picked up during the linting have been addressed and added as separate commits for clarity

NOTE

Target branch

This is targetting the drop-py37 branch. As it was assumed this PR would be merged after #435 .

Running the lint

The assumption is that pre-commit.ci GitHub integration is going to be setup for this project and therefore a dedicate lint GHA has NOT been setup, although it may look like:

---
name: Lint

on:
  push:
    branches:
      - master
      - ci
  pull_request:
    branches:
      - master
  workflow_dispatch:
    inputs: {}

jobs:
  test:
    runs-on: ubuntu-latest

    defaults:
      run:
        shell: bash

    env:
      PIP_DISABLE_PIP_VERSION_CHECK: 1

    steps:
      - uses: actions/checkout@v3
        with:
          fetch-depth: 1
          submodules: true

      - name: pre-commit
        run: |
          python -m pip install pre-commit
          python -m pre_commit run --all-files

[edgedb-cla]

All commit authors signed the Contributor License Agreement.

[adehad]

@fantix Might be an issue that tests aren't running on this PR yet (due to it not targetting the master branch).
Happy to wait for #435 to be merged first

[chrisemke]

Consideration should be given to using the following hooks to enable it in the future:
id: blacken-docs (https://github.com/asottile/blacken-docs)
id: insert-license (https://github.com/Lucas-C/pre-commit-hooks)
id: conventional-pre-commit (https://github.com/compilerla/conventional-pre-commit)

Also re-enable mypy is very important for the future

[chrisemke]

Suggested change

	- id: check-yaml
	files: .*\.(yaml|yml)$
	- id: check-yaml
	files: .*\.(yaml|yml)$
	- id: check-added-large-files
	- id: check-toml

[chrisemke]

Add the name to make it more readable and beautiful when running

Suggested change

	- id: black
	- id: black
	name: Black (Python formatter)

[chrisemke]

Add the name to make it more readable and beautiful when running

Suggested change

	- id: ruff
	- id: ruff
	name: Ruff (Python linter)

[chrisemke]

Another thing that would be really cool would be an edgedb hook to (but not limited to) verifying or formatting the .esdl and .edgeql files

[adehad]

@fantix feel free commit any of the suggestions.
(Personally I don't think the name addition to the pre-commit config adds much, I've mainly used it when I am running the same tool multiple times but with different configurations)

[fantix]

Sorry, I didn't mean to close this PR.

[adehad]

@fantix do let me know if you would like me to target the main branch instead of this drop-py37 branch

[fantix]

Yes, please! Let's retarget and rebase to the master branch, and get this merged 🙏

[adehad]

@fantix should be ready for review again now

[adehad]

Didn't realise there were merge conflicts, should now be resolved and ready for review @fantix

[adehad]

(bump) @fantix

[adehad]

@fantix have just rebased now

[fantix]

Thanks for the updates! I talked to the team and decided we don't want a pre-commit hook yet. However, I think the formatted code could stay (with a few nitpicking, for which I'll add comments on this PR) and we can probably have a test that runs the linter and formatter for checking.

[adehad]

and we can probably have a test that runs the linter and formatter for checking.

Yep, we do this too rather that as a pre-commit hook itself. I can recommend using the pre-commit.ci github action/app as this automates the process of checking PRs