feat: Enable spellchecking via crate-ci/typos by Maleware · Pull Request #649 · stackabletech/operator-templating

Maleware · 2026-09-10T10:58:41Z

This is part 1 of https://github.com/stackabletech/retro/issues/36

-> Currently a suggestion on how to approach this

Trino-Operator spike: stackabletech/trino-operator#942
docker-images spike: stackabletech/docker-images#1634

Techassi

This seems fully LLM generated and it shows.

There are just plain wrong statements, a whole bunch of unnecessary fluff, statements about well-known facts, and duplication.

In my opinion, this needs a bunch of work. I could point out all things I didn't like, but that's something I don't want to invest time for. I'm not here to review LLM-generated stuff.

I still left a few comments.

Techassi · 2026-09-25T09:39:30Z

+# check them. Turn it off so a local run and the hook agree; without it, .github/
+# and .readme/ are invisible locally but not to CI.


This is just wrong. The config is always used, independent of how typos is called (either manually (I assume that is what "local run" refers to) or via prek/pre-commit).

The same prek config is used locally and in CI. So there is literally no difference here.

It is however good to change the default of ignore-hidden.

Techassi · 2026-09-25T09:52:36Z

+        # Drop the upstream default `--write-changes` so the hook reports and
+        # fails instead of rewriting files. Keep `--force-exclude` so the
+        # excludes in typos.toml still apply to the paths prek passes in.


Suggested change

# Drop the upstream default `--write-changes` so the hook reports and

# fails instead of rewriting files. Keep `--force-exclude` so the

# excludes in typos.toml still apply to the paths prek passes in.

# Drop the upstream default `--write-changes` so the hook only

# reports failures instead of writing changes.

# Keep `--force-exclude` so the excludes in typos.toml still

# apply to the paths prek passes in.

Techassi · 2026-09-25T09:55:09Z

+  - repo: https://github.com/crate-ci/typos
+    rev: v1.50.1
+    hooks:
+      - id: typos
+        # Drop the upstream default `--write-changes` so the hook reports and
+        # fails instead of rewriting files. Keep `--force-exclude` so the
+        # excludes in typos.toml still apply to the paths prek passes in.
+        args: ["--force-exclude"]


The same comments from above apply here as well.

Maleware · 2026-09-28T06:32:40Z

Yeah, as I said in the Issue is only here to give the idea of what I want to do. It was never meant to be completed as is.

Thank you for your review however. Would you be happy on how configs are split across repositories?

…ackabletech/operator-templating into feat/enable-crate-typos-on-ci

Techassi · 2026-10-01T09:42:23Z

+
+### The core config
+
+As convention, new repositories should start from this block and add only what they actually need.


Suggested change

As convention, new repositories should start from this block and add only what they actually need.

As a convention, new repositories should start from this block and add only what they actually need.

Techassi · 2026-10-01T09:47:08Z

+# Before adding an entry here, consider an in-place marker instead. Use one when
+# the word is correct at this one site and would still be a typo elsewhere:
+#
+#   # spellchecker:disable-line       at the end of the line it applies to


I feel like this should be ignore-line instead to be in line with the ignore-next-line one.

Suggested change

# # spellchecker:disable-line at the end of the line it applies to

# # spellchecker:ignore-line at the end of the line it applies to

Techassi · 2026-10-01T09:50:29Z

+#   * unterminated `:off` suppresses nothing rather than swallowing the rest of the file.
+#   * `disable-line` only matches when the marker ends the line.
+extend-ignore-re = [
+    "(?Rm)^.*(#|//|<!--|;|/\\*)\\s*spellchecker:disable-line\\s*(-->|#\\}|\\*/)?\\s*$",


Suggested change

"(?Rm)^.*(#|//||#\\}|\\*/)?\\s*$",

"(?Rm)^.*(#|//||#\\}|\\*/)?\\s*$",

Techassi · 2026-10-02T07:27:00Z

+#   # spellchecker:disable-line       at the end of the line it applies to
+#   # spellchecker:ignore-next-line   on its own line, above the offending line
+#   # spellchecker:off / :on          around a block


On a more general note: I feel like "spellchecker" is too generic to use as a comment. My two competing thoughts on this:

It is too generic and might trigger something for some other tool

It is generic and as such, we wouldn't need to change them if we ever decide to switch tools

Overall, I would be in favour of making them more concrete, like crate-ci/typos for example.

I see your point. The intent was that if we change at some point the underlying tool, we do not have to change the naming across many repositories. I didn't find overlaps so I chose a generic approach.

Since we would need to touch that anyways, would you be fine to use typos instead? I'd like to save "-" and "/".

Techassi · 2026-10-02T07:55:54Z

+[default]
+# typos has no native suppression directive
+# (https://github.com/crate-ci/typos/issues/316), so these regexes provide one.
+# They cover `#`, `//`, `<!-- -->`, `;`, `/* */` and Jinja `{# #}` comments,


There are a couple of issues regarding this statement and the regexes below:

Using #, // or ; unterminated is fine. But if a user does decide to put anything other than whitespace behind the comment the regex no longer matches.

Some comment indicators must terminate as they would otherwise cause syntax errors. Those include , /* */, and {# #}. The regex currently doesn't enforce this.

{# #} is straight up missing from the regexes

Techassi · 2026-10-02T08:01:22Z

+`aks` is the only word that is universal.
+Everything else measured across the operator repositories turned out to be repo-local: `aas` in opa-operator, `shs` in spark-k8s-operator, base64 fixtures in secret-operator. <!-- spellchecker:disable-line -->
+Short tokens that appear in several repositories (`ot`, `fo`) do so for unrelated reasons and belong in the repository that has them, not here. <!-- spellchecker:disable-line -->


Again, do we need this extra fluff? This might have been true now, but who knows what other "universal" words there might be. The text above cleary states that this snippet is a good starting point as a per-repo config.

Techassi · 2026-10-02T08:02:13Z

+`extra/crds.yaml` is excluded on purpose.
+Its content is generated partly Kubernetes' own schema documentation and partly doc comments owned by `operator-rs` or by the operator's own `crd` module, the latter is already checked independently.


Repetition. This is already explained above. No need to explain it again.

Techassi · 2026-10-02T08:03:29Z

+
+`extra/crds.yaml` is excluded on purpose.
+Its content is generated partly Kubernetes' own schema documentation and partly doc comments owned by `operator-rs` or by the operator's own `crd` module, the latter is already checked independently.
+The same applies to files rendered from `template/`: a typo in `template/.readme/partials/borrowed/footer.md.j2.j2` is caught here, once, instead of in all sixteen repositories.


This generally makes sense to be mentioned, but not here and with reduced fluff. I suggest moving it to the top of the spell checking section and removing the example.

Techassi · 2026-10-02T08:05:22Z

+
+### Conventions
+
+- The hook runs report-only. `args: ["--force-exclude"]`.


Repetition, already explained above. Also, the missing --write-changes causes this to be run in report-only mode.

Techassi · 2026-10-02T08:07:16Z

+### Conventions
+
+- The hook runs report-only. `args: ["--force-exclude"]`.
+- Every entry in a `typos.toml` gets a one-line comment saying what the word is.


Already mentioned above, but fine to mention it again outside of the config file.

Maleware added 2 commits September 10, 2026 12:56

enable typos in operator-templating and downstream

104372a

Adding typos.toml

8fc2472

Techassi requested changes Sep 25, 2026

View reviewed changes

Maleware and others added 9 commits September 30, 2026 09:16

Merge branch 'main' into feat/enable-crate-typos-on-ci

3bf7c8a

Polishing files

4d9b209

Merge branch 'feat/enable-crate-typos-on-ci' of https://github.com/st…

87a6da7

…ackabletech/operator-templating into feat/enable-crate-typos-on-ci

Updating rev

2d7322d

making prek happy

383e20a

again ci happy dance

eee19fc

updating typo ref for template too

bd1979c

more condensed typos.toml and README.md entries

3579f4f

whitespace check

8dc6063

Techassi requested changes Oct 2, 2026

View reviewed changes

Techassi changed the title ~~enable typos in operator-templating and downstream~~ feat: Enable spellchecking via crate-ci/typos Oct 2, 2026

Maleware self-assigned this Oct 2, 2026

Applying review, change regex, more condensed README

9ae086c

		# check them. Turn it off so a local run and the hook agree; without it, .github/
		# and .readme/ are invisible locally but not to CI.

-        # Drop the upstream default `--write-changes` so the hook reports and
-        # fails instead of rewriting files. Keep `--force-exclude` so the
-        # excludes in typos.toml still apply to the paths prek passes in.
+        # Drop the upstream default `--write-changes` so the hook only
+        # reports failures instead of writing changes.
+        # Keep `--force-exclude` so the excludes in typos.toml still
+        # apply to the paths prek passes in.


		### The core config

		As convention, new repositories should start from this block and add only what they actually need.

	As convention, new repositories should start from this block and add only what they actually need.
	As a convention, new repositories should start from this block and add only what they actually need.

	# # spellchecker:disable-line at the end of the line it applies to
	# # spellchecker:ignore-line at the end of the line it applies to

	"(?Rm)^.(#\|//\|<!--\|;\|/\\)\\sspellchecker:disable-line\\s(-->\|#\\}\|\\/)?\\s$",
	"(?Rm)^.(#\|//\|<!--\|;\|/\\)\\sspellchecker:ignore-line\\s(-->\|#\\}\|\\/)?\\s$",

		`extra/crds.yaml` is excluded on purpose.
		Its content is generated partly Kubernetes' own schema documentation and partly doc comments owned by `operator-rs` or by the operator's own `crd` module, the latter is already checked independently.


		### Conventions

		- The hook runs report-only. `args: ["--force-exclude"]`.

Uh oh!

Conversation

Maleware commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Techassi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Maleware commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Maleware Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Maleware commented Sep 10, 2026 •

edited

Loading

Maleware commented Sep 28, 2026 •

edited

Loading

Maleware Oct 2, 2026 •

edited

Loading