Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk.
3 comments:
Commit Message:
Patch Set #1, Line 7: Make penable cb more descript
It's not a callback anyway. It's a function pointer used for […]
Reading a bit too deeply into such a small change are we not, I guess I went a bit far with such detail in the commit message myself. I only said "cb" to try and keep the short line short. I went with 'fn ptr' instead as Nico points out its more semantically correct to say its a function pointer given the control flow.
typo use*d*
Done
File programmer.h:
Patch Set #1, Line 228: enable_flash_xxx
How about `run` or anything else that doesn't repeat `enable`.
I think I much prefer the current pattern as "_xxx" as, in programming 'xxx' is typically recognised as a find-and-replace pattern and thus, it makes it super easy to identify that `enable_flash_sis5511()` is a concrete realisation for an example.
Any other name incurs more mental indirection which entirely is the motivation of the change to remove. It is easy for us given that we are familiar with the code however imagine someone who has never looked at Flashrom before, see's this struct, greps the field name or struct identifier. The developer would very quickly conclude the connection with the consistent naming without any more effort than a pattern match.
TL;DR the mundane `enable_flash_xxx` identifier leads to both consistent naming and minimal mental indirection with maximal grep friendliness.
To view, visit change 52364. To unsubscribe, or for help writing mail filters, visit settings.