NetLay: Writing it down for the next person

A manual for the code#

The app has a guide now. The code has nothing, and the code is about twenty one and a half thousand lines.

"Comment each line with a human readable comment."

That was the ask, and I did not do it literally, because commenting every line doubles the size of every file and turns count += 1 into increment the counter, which is worse than silence. Noise does not become documentation by being adjacent to code.

What I did instead: a header on every file saying what it is for, a comment on every function saying what it does and why, and a comment above each block of a few lines that is doing something non-obvious. Tests left alone, because a test that needs explaining should be rewritten rather than annotated.

What a comment is for.#

The rule I kept coming back to is that a comment explains the why and never the syntax. # loop over the devices tells you what you can already see. # a device that still answers on its current address has not moved, whatever else its MAC is doing tells you about a bug from post twenty four that you would otherwise reintroduce.

Jargon gets defined once, at the top of the file that uses it. ARP, VLAN, SNMP, OID. That is for the person who is a good programmer and not a network person, who is precisely the sort of person who might otherwise change something subtle in the tracker.

My favourite of the lot is one line in the lease parser noting that an expiry of zero in dnsmasq means an infinite lease. Nobody would guess it, it is not obvious from the code, and getting it wrong silently expires every static lease in the file.

Proving it changed nothing.#

Here is the part that made this safe, because a three thousand line change to working code is terrifying otherwise.

Three checks, all automated, all run before anything was committed.

The diff contains only added lines, and every added line is a comment. If anything else appears in the diff, stop.

Python's own syntax tree is identical before and after. Parse the file both ways, compare the trees, and any change in actual behaviour shows up immediately.

And the TypeScript compiles to byte identical output once comments are stripped. This is the one that earns its keep, because of a specific trap: a double slash typed inside JSX is not a comment, it is text that will appear on the screen. The compiler will not complain, the tests will not fail, and the user sees // the member list in the middle of a panel.

The first version of that checker was too strict and flagged perfectly good changes, because JSX block comments and comments sitting before object properties survive compilation in a way that moves the output around. Comparing the minified output fixed it.

Reading your own code closely.#

The side effect I did not anticipate is the real result of this work.

You cannot write an honest comment about a block without understanding it, and understanding a block you wrote six weeks ago means noticing what is wrong with it. I ended up with a list of about twenty five possible bugs, found purely by having to describe things.

Three worth repeating. Anything made by Hewlett Packard gets classified as a PC, because the rules table matches on "HP" before it gets to the printer rules. A run now request is simply lost if a poll is already in progress, rather than queued. And any signed in user can start the DHCP tracker, which should be an admin job like the alerts and the ping settings are.

Every one of those is still unfixed on purpose. This change was three thousand comments and nothing else, and mixing fixes into it would have destroyed the one property that made it safe to make, which is that it provably does nothing.

They are on the list. Which is the other thing a close read gives you, a list that came from reading rather than from waiting for somebody to complain.

Next time, the last one in this run, and the one where the network team finally sent me something.

NetLayNetworking