Although I might disagree with the style of the write up, I think it is right that there are weak engineering practices applied:
- not enough information provided in commit message. - this change ought to have been split into smaller chunks (e.g. introduce indirection whilst code continues to use hardware) - limited (automated) testing - potential confusion by the author over C concepts - additional complexity which could have been hidden behind a cleaner interface rather than using defines
I agree about your points about:
- defines over multiple repo's made this more complicated that was necessary. - fault injections would have been the appropriate way to test this (or maybe mocks) - testing of randomness needs careful design, e.g. RNG seeding can be a good way
Taking a step back, we as a community of excellence need to emphasise that this isn't a criticism about the person/author. There needs to be a clear analysis of what went wrong whilst being kind to the person/people involved.