Skip to content

feat: classify tech debt into weak and strong#28

Merged
jcommelin merged 1 commit intomasterfrom
weak-strong-tech-debt
May 4, 2026
Merged

feat: classify tech debt into weak and strong#28
jcommelin merged 1 commit intomasterfrom
weak-strong-tech-debt

Conversation

@Vierkantor
Copy link
Copy Markdown
Contributor

This PR adds a "level" attribute to the tech debt reports posted weekly on Zulip and under every PR. The intention is that "weak" tech debt is allowed to increase in a PR (when reasonable), while "strong" tech debt should only increase in exceptional cases (for example, Lean updates or because we expose a new source of tech debt).

I played around with a few formatting options for the report and settled on this one: show the strong tech debt and then the weak tech debt in two different tables. That should make it clear for the reader (and possible automation) if we care about a particular change or not.

This PR adds a "level" attribute to the tech debt reports posted weekly on Zulip and under every PR. The intention is that "weak" tech debt is allowed to increase in a PR (when reasonable), while "strong" tech debt should only increase in exceptional cases (for example, Lean updates or because we expose a new source of tech debt).

I played around with a few formatting options for the report and settled on this one: show the strong tech debt and then the weak tech debt in two different tables. That should make it clear for the reader (and possible automation) if we care about a particular change or not.
Copy link
Copy Markdown

@grunweg grunweg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two comments about the classification of tech debt counts. (I did not look at the shell/awk changes carefully.)

grep -c "deprecated .*(since")"

printf '%s|disabled deprecation lints\n' "$(( deprecs - doubleDeprecs ))"
printf '%s|disabled deprecation lints|strong\n' "$(( deprecs - doubleDeprecs ))"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as lean#13595 is not merged yet, this check will have false positives. Perhaps that's fine for a two-week window, but perhaps this should start as "weak" (and be changed to "strong" in two weeks)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be easy enough to change the category later, so:

Suggested change
printf '%s|disabled deprecation lints|strong\n' "$(( deprecs - doubleDeprecs ))"
printf '%s|disabled deprecation lints|weak\n' "$(( deprecs - doubleDeprecs ))" # TODO: upgrade to strong after false positives are fixed, https://github.com/leanprover/lean4/pull/13595

printf '%s|%s|weak\n' "$(git grep '^set_option linter.style.longFile [0-9]*' Mathlib | wc -l)" "large files"

printf '%s|%s\n' "$(wc -l < scripts/nolints_prime_decls.txt)" "exceptions for the docPrime linter"
printf '%s|%s|strong\n' "$(wc -l < scripts/nolints_prime_decls.txt)" "exceptions for the docPrime linter"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this list is effectively not updated, I'm unsure if this reflects reality. (But it's not actively doing harm either.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good point. Maybe we should instead count the set_option linter.docPrime false lines?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I mean, adding the line count of this file to the count of set_option linter.docPrime false. The list is used by the linter for skipping exceptions.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can count just set_option linter.docPrime false (these shouldn't be introduced)... but ultimate, I don't think this items needs much thought right now, given the linter is disabled.

@grunweg
Copy link
Copy Markdown

grunweg commented May 3, 2026

Also, let me say thank you for starting this --- I think this can be very useful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants