Hacker News Reader

Comment by Kwpolska | Hacker News Reader

Looks like a case of print() debugging making it into production:

https://github.com/gnachman/iTerm2/commit/63ec2bb0b95078a97a... https://github.com/gnachman/iTerm2/blame/5db0f74bf647f6d53ea...

View on HN

That's not unreasonable code in itself, it's writing to the file only if verbose mode is enabled.

This is the commit which disabled verbose mode, just before the code which removed verbose framer logging entirely: https://github.com/gnachman/iTerm2/commit/014ba7ec40fc790f65...

This is the commit which enabled VERBOSE mode: https://github.com/gnachman/iTerm2/commit/5db0f74bf647f6d53e... (from Jul 3, 2024)

That is probably just from having set VERBOSE=1 while implementing or debugging something and forgetting to revert it to VERBOSE=0 before committing.

This is why all "temporary" settings go in .gitignored files or equivalent,

or on no-merge testing branches,

and we have test on the release branch that asserts for each config value that it is set to a safe value,

and we have a test that reflectively asserts that every config key has a value assertion test.

Or non-default compile time flags. That’s how I handle verbose debugging in my terminal emulator.

This variable might have been better named LOG_ALL_STDIO_TO_TMP. Would have made it more obvious during self-review that it shouldn't be shipped.

Or even better:

# TODO: set back to 0

This appears as a warning in my editor, and of course is greppable

Don't mind me with 100s of // TODO in my codebases.

Then # HACK: or # FIXME: or # DO_NOT_SHIP... anything greppable and distinct enough to error in CI

or having a commit hook that fails if the code contains such string

This happened to me so often that I made a git pre-commit hook to catch it: https://github.com/nobssoftware/nocommit for the word NOCOMMIT

Includes a GitHub CI action to prevent merging it in to master

It has caught so many of these mistakes for me…

I only heard of pre-commit hooks like 20 times in the wild and I really want to emphasize how badly that number should be 10x larger. Got a team completely unaware they existed once.

They are just not convenient to use since they are per repo and not shared via clone

They can be configured globally with core.hooksPath or init.templatedir git config options.

I really do wish that git itself would like check for a `.githooks` directory and prompt if it should set `core.hooksPath` to that path (probably would need to notify on pull when any files in it changed for security reasons)

I would also love if it detected a `.gitmessage` for message the message template without needing to set it up for each repo, but that is a different matter.

In typescript dev I made “console.log” a linting error that cannot be merged. The occasional legitimate need uses console.info

I think print debugging is fine. It has a time and place. But ideally find a way to protect yourself from accidentally leaving it in. It’s such an easy mistake to make.

Well this is an instance of writing to a file, so the JS equivalent would be calling .write on a file stream. Not exactly comparable, it wasn't literally a debug print() that someone had forgotten to delete but a log file that was enabled when verbose logging was enabled.

in similar situations, instead of saying "VERBOSE=1", I say "VERBOSE=getenv("MY_NAME_MY_APP_VERBOSE") == '1'", and set this env variable in my terminal when needed. This way there is zero chance I commit verbose-enabled debug code.

not a typescript dev here: what do you use instead of console.log for debugging?

It sounds like they use console.log, but they don't let it ship in their production code.

I liked tslog last time I tried it.

console.log is reserved for devtime debugging and is a linting error so it cannot be left in. console.info is used in the occasional time we actually want to write things to console (along with console.warn, console.error, console.group, console.table, etc.).

It's all it took. Just making a clear distinction between the two and communicating it with the team.

It’s been around for 3 years?

About six months. File was originally authored a few years back, but looks like this slipped in here: https://gitlab.com/gnachman/iterm2/-/commit/5db0f74bf647f6d5...

Disabled by default until 7 months ago.

Reply on news.ycombinator.com