Add precommit for linting and formatting #9123
Conversation
kdaily
force-pushed
the
kdaily-add-precommit-hook
branch
6 times, most recently
from
85393a5 to
f932015
Compare
kdaily
force-pushed
the
kdaily-add-precommit-hook
branch
4 times, most recently
from
0fb1fd7 to
6156454
Compare
kdaily
force-pushed
the
kdaily-add-precommit-hook
branch
from
6156454 to
46cb394
Compare
| @@ -0,0 +1,33 @@ | |||
| exclude: "\ | |||
| ^(\ | |||
| .changes|\ | |||
There was a problem hiding this comment.
Non-blocking: should awscli/data, be on this list?
There was a problem hiding this comment.
I think we should leave that directory in. There are only JSON files in it, and they should be subject to the end of file fixer check; the rest of the checks aren't applicable anyways.
Indeed, running the basic linting catches that one file does not have a trailing newline:
There was a problem hiding this comment.
Does the linter actively change these? If they're getting re-updated with each release, we should avoid them.
There was a problem hiding this comment.
Does the linter actively change these?
It only adds the single new line that was missing. cli.json contains the top level documentation descriptions, it is not changed in each release. metadata.json was only added for additional metadata in the user agent string.
pyproject.toml
Outdated
| line-length = 79 | ||
| indent-width = 4 | ||
|
|
||
| target-version = "py38" |
There was a problem hiding this comment.
Is this still right? Are we waiting until botocore raises pins? I think the CLI v2 can be a little more aggressive.
There was a problem hiding this comment.
I assumed since our minimum version is Python 3.8, thats what we would have here. After a closer reading of the ruff docs, it would effectively be set to py38 because we have this in our pyproject.toml too:
Line 16 in d4619a3
From the docs:
If you're already using a pyproject.toml file, we recommend project.requires-python instead, as it's based on Python packaging standards, and will be respected by other tools. For example, Ruff treats the following as identical to target-version = "py38":
[project] requires-python = ">=3.8"
There was a problem hiding this comment.
Cool, we can probably yank this then and let requires-python be the single source of truth.
kdaily
force-pushed
the
kdaily-add-precommit-hook
branch
from
46cb394 to
252fa9b
Compare
First pass at adding a pre-commit hook to enforce
formatting and linting standards.
This adds the pre-commit command to run on a
limited set of directories. Subsequent changes
will remove excluded folders and run formatting on
them as well.
The command will use the standard pre-commit hooks
for yaml, whitespace, and end of line linting.
It also uses `ruff` configured identically to
the develop branch, except for disabling
unused import checks ("F401"), as there are known
imports that are unused in the module but imported
in other modules. This is common in `compat` but
in other places as well. It also adds the `I` flag
to perform import sorting.
The intention is to start enforcing formatting and
linting standards on edited code. Running the
pre-commit hook manually will make numerous
changes that should be broken into multiple
phases. Additional PRs will work to get the
codebase up-to-date passing all standards.
kdaily
force-pushed
the
kdaily-add-precommit-hook
branch
from
252fa9b to
0cb6373
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v2 #9123 +/- ##
==========================
==========================
☔ View full report in Codecov by Sentry. |
Description of changes:
First in a series of commits to lint and format the
v2branch of this repository.Adds a precommit hook configuration file and enables whitespace linting,
isortfor imports, andruffto lint and format. It adds configuration topyproject.tomlforruffas well with the same configuration as thedevelopbranch, with the exception of ignoring unused imports (F401). This exception will be dropped at a later date.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.