Add pre-commit #82
Add pre-commit #82
Conversation
This commit **does not** run pre-commit on the repo, it just adds it as a dev dependency, and adds the following checks. - prevents commits to the `main` branch - remove unused imports from python files - modifies all imports to be absolute for python files - removes commented out code in python files - runs `isort` on imports for python files - runs `black` on python files (formatting) - runs `ruff` on python files (linting) - fixes for the `setup.cfg` file - ensures `json`, `yaml`, and `toml` files are parseable (ignores the conda.recipe/meta.yaml file) - uses local installs of eslint and prettier for linting and formatting any [java|type]script files. Also, three additional files have been added. - `ipywidgets_bokeh/.eslintrc.js` - `ipywidgets_bokeh/.prettierrc` These files contain the formatting and linting rules as outlined in Bokeh. The last file is a `project.toml` file that for the time being only contains preferences for `ruff`. In the future we can use it for complying with `setuptools` installs. The `package*.json` files have also been modified to include eslint and prettier, as well as updating the `lint` script from a `TODO` to an actual linting action.
Noticed that two packages used for testing linting and formatting on [Java|Type]Script files were left in the package.json file. These have been removed as `pre-commit` is handling these checks.
The CI failures are associated with the `lint` script in `package.json`. This commit removes the new command, as we have not run the new pre-commit lints and formatters on the codebase yet. It also adds `rimraf` as a dev dependency in `package.json`, as it was previously missing.
|
Happy to see this merged! |
| - python | ||
| # Lint Python files. | ||
| - repo: https://github.com/charliermarsh/ruff-pre-commit | ||
| rev: v0.0.242 |
There was a problem hiding this comment.
Can this be set up via an environment file instead of separately here?
| - --verbose | ||
| - --diff | ||
| - --respect-gitignore | ||
| - --line-length=88 |
There was a problem hiding this comment.
This is pretty outdated limit and inconsistent with bokeh. We set this up higher in bokeh and bokehjs (~165) to avoid unnecessary line splitting. The same applies to setting up other tools, e.g. prettier.
There was a problem hiding this comment.
We definitely want to mimic what Bokeh does, and I'll update this to be 165
| types_or: | ||
| - python | ||
| # Lint Python files. | ||
| - repo: https://github.com/charliermarsh/ruff-pre-commit |
There was a problem hiding this comment.
Is this and similar repositories trustworthy enough? This is a lot of repositories that I don't know anything about.
There was a problem hiding this comment.
That is a very good question. We could run bash scripts for the different linting/typing/formatting tools if a dev has them installed. Let me experiment with this idea more.
This commit does not run pre-commit on the repo, it just adds it as a dev dependency, and adds the following checks.
mainbranchisorton imports for python filesblackon python files (formatting)ruffon python files (linting)setup.cfgfilejson,yaml, andtomlfiles are parseable (ignores the conda.recipe/meta.yaml file)Also, three additional files have been added.
ipywidgets_bokeh/.eslintrc.jsipywidgets_bokeh/.prettierrcThese files contain the formatting and linting rules as outlined in Bokeh.
The last file is a
project.tomlfile that for the time being only contains preferences forruff. In the future we can use it for complying withsetuptoolsinstalls.The
package*.jsonfiles have also been modified to include eslint and prettier, as well as updating thelintscript from aTODOto an actual linting action. This helps with the requests in #75Resolves #80