Message ID | 20210613063702.269816-1-felipe.contreras@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | Trivial cleanups | expand |
Felipe Contreras <felipe.contreras@gmail.com> writes: > These perfeclty good patches from 2014 weren't picked with no good > reason. These are safe no-op changes, but that does not mean they are good patches. It goes against Documentation/CodingGuidelines to bring it back again now, which is a good enough reason not to look at them.
Junio C Hamano wrote: > Felipe Contreras <felipe.contreras@gmail.com> writes: > > > These perfeclty good patches from 2014 weren't picked with no good > > reason. > > These are safe no-op changes, but that does not mean they are good > patches. They are not good patches because they are no-op, they are good changes because they correct the flow of the code to match the flow in which most people think. 18 is younger than Mary is not a sentence most people would agree make sense. > It goes against Documentation/CodingGuidelines to bring it > back again now, which is a good enough reason not to look at them. These are not style issues. Refactoring code to make it easier to understand goes beyond style. Refactoring this: static int is_same_remote(struct remote *remote) { struct remote *fetch_remote = remote_get(NULL); return (!fetch_remote || fetch_remote == remote); } int same_remote = is_same_remote(remote); To this: int same_remote = remote == remote_get(NULL); Is more than just style. But suit yourself. Sooner or later somebody is going to fix these glaring mistakes. And they are mistakes [1]. Yoda conditions are widely criticized for compromising readability by increasing the cognitive load of reading the code. Cheers. [1] https://en.wikipedia.org/wiki/Yoda_conditions#Criticism