If I feel like the author might just not be aware of some "better" (subjectively) way of doing things, then I'll leave a "FYI: could also do this in X way" type of comment.
If I feel like the author might just not be aware of some "better" (subjectively) way of doing things, then I'll leave a "FYI: could also do this in X way" type of comment.
Personally, I would prefer someone actually make the suggested fix they're describing - doing the actual code - so I could see it fleshed out. Commenting "use a generatorInterface" to someone who's probably not used it before is less helpful than actually implementing the generatorInterface in the PR and then discussing the actual code. Not always time for that, but if it's never considered, there's never any time for that approach.
tanget: I had a PR blocked because... "use more descriptive variable name" was applied to a 'for (i=0; i<upperBound; i++) ...' loop. The complaint was about using 'i' in the for loop. This was in a test file - the first test file on the project that had been live for 7 months - and I think this nitpick was just a bit over the top. And it wasn't enforced later, just ... someone making a stink over 'a new guy' joining and trying to exert some influence.