[Pre-commit] Expand Pre-commit Hooks #213

pishoyg · 2024-08-23T19:13:15Z

Time for some developer experience improvements!

I don't know exactly what was wrong with the `Object.entries(...).forEach(([key, value]), () => {});` syntax, but `fixmyjs` kept complaining about an `Unexpected token =>` for this loop in particular, so we changed the loop to make it happy.

`jshint`: - Make `.jshint` configuration stricter. Notice that we disabled some of the rules that we can't force the TypeScript compiler to respect. `mypy`: - We disable `mypy` (temporarily) because it requires extensive fixes. Filed #215 to reenable it. - Ignore `.mypy_cache` in `.gitignore`, `stats.sh`, and `.helpers`. YAML Hooks - Add `yamllint` hook. - Add a `.yamlfmt` and `.yamllint` configuration files. Account for them in `stats.sh`. The files were required in order to make the two linters compatible. `.pre-commit-config.yaml` - Reformatted the file to accommodate the new linter requirements: - Add a document start, denoted by `---`. - Set the maximum line length to 80. Rewrite many strings, e.g. by removing the unnecessary quotes, or using the block scalar (denoted by `>` or `|`), or single quotes in order to enable splitting strings to multiple lines. NOTE: https://yaml-multiline.info/ explains YAML strings well. - Rewrote many of the `bash` scripts contained within the file, thanks to the newly-discovered YAML string powers: - Include `set -o nounset` in many of the `bash` scripts. - NOTE: check-do-not-submit` has been fixed. Fix #199. - Delete some redundant `types` specifications. - Delete the `.git` suffix from The paths of a couple of repos. - Changed the TODO about fail-fast to a NOTE because there is currently no action required. - Changed the TODO about installing more hooks from https://pre-commit.com/hooks.html to make it Python-specific, because we already acquired most of the other commits. `json-tool`: - Run on `.csslintrc` and `.jshintrc` as well. `typos`: - Add a new spell-checking hook. (It was problematic, but we're trying to retain it.) Security Hooks: - We add a few hooks that check for leaked secrets. Misc: - Where possible, add links to the configuration pages inside the hook config files. The links don't need to be added to `.pre-commit-config.yaml`, because it already has links to the rpos (albeit not directly to the config pages). But it's helpful to have links in such files as `.yamlfmt`, `.yamllint`, and `eslint.config.mjs`.

- Run most bash hooks with stricter options whenever possible. - Fix bug in commit-file-sizes. Use `"${@}"` in order to be able to handle files with spaces in their names. - Delete redundant `pass_filenames: true`. This is the default value. - Delete redundant `always_run: true`. This is the default behaviour when there are no filters. - Refactor `check-one-readme` to look only at new / modified files, rather than all files in the repo. - Some commands fail when `"${@}"` is empty, so we check for `"$#" == 0` before proceeding. This may be unnecessary in many cases, but we do it anyway. - `check-do-not-submit` mangles the regex instead of excluding `.pre-commit-config.yaml`.

pishoyg · 2024-08-25T09:34:43Z

TODO: Likely, the only thing left for this is adding some Python hooks (see). We want to add them (as we did for many others) just because they're recommended on pre-commit.com!

I don't think we will have anything else to do for this issue afterwards.

NOTE: There is also #215, which concerns mypy. It has its own issue because work for it seemed substantial.

This is to make it compatible with other Python tools that we are about to adopt.

Pick up the new line length limit.

Side change: - Remove an override of the name of `isort`.

The biggest change is the addition of trailing commas which, in combination with Black's interpretation of trailing commas, results in many parameter lists being spread over several lines instead of being compactly pressed on one line.[1] [1] https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma

It conflicts with a standard library package[1], so our linters and formatters get really confused by it! [1] https://docs.python.org/3.9/library/parser.html

- Delete some dead code. - Disable `dead` in some locations using `# dead: disable`.

The helper was introduced in 5fcee4c when the severity of the `dead` pre-commit was reduced to warning.

It's very unlikely to be used, given that we always mirror our GHSEET files to a local TSV before using them.

This picks up the updated email (#98). It also picks up a small change from f24753d. This should be all.

pishoyg · 2024-09-02T13:27:04Z

We were about to adopt this config for Tidy:

break-before-br: yes
enclose-block-text: yes
enclose-text: yes
indent-spaces: 2
indent: auto
join-classes: yes
priority-attributes: id,name,class
punctuation-wrap: yes
quiet: yes
sort-attributes: alpha
strict-tags-attributes: yes
tidy-mark: no
vertical-space: yes
wrap: 80
write-back: yes

NOTE: It caused some issues with the site_publish pipeline, so we reverted it!

- Unset `noreturnawait` for `jshint`. I don't fully understand the utility of this flag at this point, but it's causing some trouble. I don't know whether it's worth appeasing this flag, so I will simply unset it. - Add `Response` as a known global. P.S. `jshint` is so far useless, and hard to appease.

It's creating toil, but has no value. We don't write JavaScript in the first place. We write TypeScript, and transpile it to JavaScript.

We open raw HTML files often. Prettifying them even further will be helpful to us. It's a huge commit, but we don't have concerns about commit sizes at the moment.

See the comment in `pre-commit-config.yaml` about fail fast.

The bug was introduced in 754d887. We moved the hook without moving its `exclude` parameter along with it.

- Introduce a new script, `dictionary/marcion.sourceforge.net/appendices.py`, that is responsible for all the appendices-related actions. Currently, the actions are downloading the sheets into the local mirrors, and validating the content. - The script replaces the following: - The Makefile recipe that downloads the sheets and performs some basic validation. The recipe has been replaced with a new recipe that simply calls the script mentioned above. - The logic in the Flashcards pipeline doesn't need to perform any validation, it can take the integrity of the data for granted thanks to the pre-commit hooks. Python is more versatile and suitable for this use case. This definitely didn't belong in a Makefile recipe! This is also consistent with the pattern that we have followed for Crum images.

This is the (main) reason they are not compatible: CSSLint/csslint#792.

The `clean-css` Hook was added in 94b69ae. It was implemented in JavaScript, not TypeScript, because the TypeScript package[^1] is outdated. We required a `clean-css` version >= 5.0 for our use case; in particularly, we want to assign numerical values for line breaks, not just a boolean (see `pre-commit/clean-css.js`). [^1] https://www.npmjs.com/package/@types/clean-css

Consistency is desirable.

The name is different on this machine, for some reason!

It was added in 21dd272, but we didn't include installation steps in `Makefile`.

Notice that this requires modifying the `eslint` config in order to avoid conflict with the new hook.

`prettier` already supports JSON. There is no need for us to maintain our own hook. Indeed, `prettier` does a better job at producing elegant JSON than `json-tool`.

pishoyg added the dev Why: Developer experience label Aug 23, 2024

pishoyg added this to the Developer Experience milestone Aug 23, 2024

pishoyg self-assigned this Aug 23, 2024

pishoyg added a commit that referenced this issue Aug 23, 2024

[#213][Pre-commit] Add Hooks from github.com/pre-commit/pygrep-hooks/.

50e1cbf

pishoyg added a commit that referenced this issue Aug 23, 2024

[#213][Pre-commit] Run pre-commit run --all-files.

431e6d9

pishoyg added a commit that referenced this issue Aug 23, 2024

[#213][Python/Pre-commit] Add the mypy Pre-commit Hook.

269badb

pishoyg added a commit that referenced this issue Aug 23, 2024

[#213][Pre-commit] Nit: Add Language Names / Targets in Comments.

0b307c7

pishoyg added a commit that referenced this issue Aug 23, 2024

[#213][Pre-commit] Add jshint, and Appease it.

4130ff4

pishoyg removed their assignment Aug 24, 2024

pishoyg added a commit that referenced this issue Aug 24, 2024

[#213][Pre-commit] Add some Meta Pre-commits.

d30fe68

pishoyg added a commit that referenced this issue Aug 24, 2024

[#213][Pre-commit/README] Show the pre-commit badge in your README.

69099ee

pishoyg added a commit that referenced this issue Aug 26, 2024

[#66,#213][TODO/Pre-commit] Retain Colors from grep in check-todo.

51f34fb

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Pre-commit] Restrict Execution of some Custom Hooks with types.

cf6a747

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Python/Pre-commit] Set Black Max Line Length to 79.

02a905a

This is to make it compatible with other Python tools that we are about to adopt.

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Python/Pre-commit] Run pre-commit run black --all-files.

c138eb1

Pick up the new line length limit.

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Python/Pre-commit] Add Python Hooks.

ad6cedd

Side change: - Remove an override of the name of `isort`.

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Python/Pre-commit] Rename parser to parse.

3bfcc45

It conflicts with a standard library package[1], so our linters and formatters get really confused by it! [1] https://docs.python.org/3.9/library/parser.html

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Python/Pre-commit] Handle some dead Catches.

012375e

- Delete some dead code. - Disable `dead` in some locations using `# dead: disable`.

pishoyg closed this as completed in 5fcee4c Aug 26, 2024

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Python/Pre-commit] Color the Output of the pydead Helper.

632a620

The helper was introduced in 5fcee4c when the severity of the `dead` pre-commit was reduced to warning.

pishoyg mentioned this issue Aug 26, 2024

[Python] Stop Using Global Variables #219

Open

pishoyg added a commit that referenced this issue Aug 26, 2024

[#213][Crum/Flashcards] Delete the gsheet field type.

003bd85

It's very unlikely to be used, given that we always mirror our GHSEET files to a local TSV before using them.

pishoyg modified the milestones: Developer Experience, Platform Aug 26, 2024

pishoyg added a commit that referenced this issue Aug 27, 2024

[#213][Pre-commit] Ignore .ruff_cache.

57a8834

pishoyg added a commit that referenced this issue Sep 1, 2024

[#98,#213,#66][Crum/Flashcards] Run make flashcards.

fa75211

This picks up the updated email (#98). It also picks up a small change from f24753d. This should be all.

pishoyg added a commit that referenced this issue Sep 3, 2024

[#213][Tidy] Retrieve Tidy Config from a File.

0ccf247

pishoyg added a commit that referenced this issue Sep 3, 2024

[#213][JavaScript/Pre-commit] Add fetch and alert as Known Globals.

e26fc68

pishoyg added a commit that referenced this issue Sep 4, 2024

[#213][JavaScript/Pre-commit] Remove the jshint Hook.

c8a606c

It's creating toil, but has no value. We don't write JavaScript in the first place. We write TypeScript, and transpile it to JavaScript.

pishoyg added a commit that referenced this issue Sep 4, 2024

[#213][HTML/Pre-commit] Update Tidy Config.

6b579d2

We open raw HTML files often. Prettifying them even further will be helpful to us. It's a huge commit, but we don't have concerns about commit sizes at the moment.

pishoyg added a commit that referenced this issue Sep 4, 2024

[#213][HTML/Pre-commit] Run pre-commit run --all-files tidy-html.

5ce9d85

pishoyg added a commit that referenced this issue Sep 4, 2024

[#213][Pre-commit] Execute trailing-whitespace Earlier.

754d887

See the comment in `pre-commit-config.yaml` about fail fast.

pishoyg added a commit that referenced this issue Sep 5, 2024

[#213][Pre-commit] Fix Bug in trailing-whitespace.

f5b4fc3

The bug was introduced in 754d887. We moved the hook without moving its `exclude` parameter along with it.

pishoyg added this to coptic Sep 11, 2024

pishoyg added a commit that referenced this issue Sep 14, 2024

[#213][CSS/Pre-commit] Allow Headings to Be Defined Multiple Times.

a78f486

pishoyg mentioned this issue Oct 3, 2024

[Crum/Flashcards] Organize the HTML #233

Open

pishoyg added a commit that referenced this issue Oct 8, 2024

[#213][CSS/Pre-commit] Add stylelint and remove csslint.

21dd272

This is the (main) reason they are not compatible: CSSLint/csslint#792.

pishoyg added a commit that referenced this issue Oct 8, 2024

[#213][CSS/Pre-commit] Appease stylelint.

a4bc85a

pishoyg added a commit that referenced this issue Oct 8, 2024

[#213][CSS/Pre-commit] Add clean-css Hook.

94b69ae

pishoyg added a commit that referenced this issue Oct 8, 2024

[#213][CSS/Pre-commit] pre-commit run clean-css --all-files.

c9949be

pishoyg added a commit that referenced this issue Oct 9, 2024

[#213][CSS/Pre-commit] Always Use Integer Values for breaks.

ec2b9b9

Consistency is desirable.

pishoyg added a commit that referenced this issue Oct 9, 2024

[#213][CSS/Pre-commit] Add a Comment Explaining why we use JavaScript.

c3d801c

pishoyg added a commit that referenced this issue Oct 17, 2024

[#213][Python/Pre-commit/Git] Change the mypy Cache Location.

5b030c7

The name is different on this machine, for some reason!

pishoyg added a commit that referenced this issue Oct 21, 2024

[#213][CSS/Pre-commit] Add Installation Steps for stylelint.

10a73bf

It was added in 21dd272, but we didn't include installation steps in `Makefile`.

pishoyg added a commit that referenced this issue Nov 27, 2024

[#213][TypeScript/pre-commit/eslint] npm add --save-dev Packages.

1826026

pishoyg added a commit that referenced this issue Nov 27, 2024

[#213][TypeScript/pre-commit] Add the prettier Hook.

5284b77

Notice that this requires modifying the `eslint` config in order to avoid conflict with the new hook.

pishoyg added a commit that referenced this issue Nov 27, 2024

[#213][TypeScript/pre-commit] pre-commit run prettier --all-files.

f9ca38a

Notice that this requires modifying the `eslint` config in order to avoid conflict with the new hook.

pishoyg added a commit that referenced this issue Nov 27, 2024

[#213][JSON/pre-commit] Use prettier for JSON.

752d492

`prettier` already supports JSON. There is no need for us to maintain our own hook. Indeed, `prettier` does a better job at producing elegant JSON than `json-tool`.

pishoyg added a commit that referenced this issue Nov 27, 2024

[#213][JSON/pre-commit] Run pre-commit run prettier --all-files.

5488f86

pishoyg added a commit that referenced this issue Nov 27, 2024

[#213][TypeScript/pre-commit] Use prettier for CSS.

3d6e655

pishoyg added a commit that referenced this issue Nov 27, 2024

[#213][JSON/pre-commit] Use prettier for JSON.

672aa25

`prettier` already supports JSON. There is no need for us to maintain our own hook. Indeed, `prettier` does a better job at producing elegant JSON than `json-tool`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Pre-commit] Expand Pre-commit Hooks #213

[Pre-commit] Expand Pre-commit Hooks #213

pishoyg commented Aug 23, 2024

pishoyg commented Aug 25, 2024

pishoyg commented Sep 2, 2024 •

edited

Loading

[Pre-commit] Expand Pre-commit Hooks #213

[Pre-commit] Expand Pre-commit Hooks #213

Comments

pishoyg commented Aug 23, 2024

pishoyg commented Aug 25, 2024

pishoyg commented Sep 2, 2024 • edited Loading

pishoyg commented Sep 2, 2024 •

edited

Loading