ENB: Improving mypy annotation will change all of Leo's core files

[Edward K. Ream]

​This Engineering Notebook post discusses requiring strict mypy annotations for all of Leo's core files. Félix and I agree that the benefits far outweigh the (real!) risks of creating new bugs.

tl;dr: I made some mistakes in the initial PR, but trying to undo those mistakes creates more serious problems. I've learned from those mistakes—follow-on PRs will be more disciplined.

Background

First, let's put one mistaken notion to rest. No matter how we split the work, this project will change thousands of lines. We expect that most of those changes will affect only annotations, but the work has already exposed dubious executable code, including at least one significant bug. It is pointless to complain about the size of the diffs. Furthermore, Leo's git-diff-pr makes it much easier to examine diffs calmly.

PR #4755 began the work, and it quickly became controversial. It could rightly be said to be too large. It's also untidy :-) It contains unfinished work that is spread over seemingly unrelated files.

Yesterday, I experimented with splitting this PR into 5 "incremental" PRs, each focused on one particular file in Leo's core: leoGlobals.py, leoNodes.py, leoCommands.py, leoKeys.py, and leoApp.py. But this experiment created new problems, as explained next.

Splitting the initial PR was a bad idea

This morning, I started fixing bugs reported against several of the smaller PRs. I quickly realized that each fix creates an unpleasant administrative chore. Either I fix the bug in all of the incremental PRs, or I must fix (later) any merge conflicts that may arise.

You may think that such administrivia is a minor matter. But you would be mistaken. There are already too many ways to go wrong. I don't want to add another.

Worse, there is a fundamental defect in the incremental PRs. They began by taking one core file from the first PR. Alas, this deletes the commit history. Checking out the base file from the ekr-strict branch forms a single commit. But I absolutely do not want to lose that commit history! Days, weeks, or months later, we might want to do a git bisect to find the source of a subtle bug. The more detailed the commits are, the easier it will be to zoom in on the offending lines.

Summary

Yes, PR #4755 is too large for comfort. It would have been better to focus on one core file at a time. Yes, that was a mistake, but it's not particularly serious.

I've learned from my recent experiences. Follow-on PRs will be more disciplined.

I have reopened  PR #4755 and will soon close the incremental PRs.

All questions and comments are welcome.

Edward