mbox series

[0/2] Trivial cleanups

Message ID 20210613063702.269816-1-felipe.contreras@gmail.com (mailing list archive)
Headers show
Series Trivial cleanups | expand

Message

Felipe Contreras June 13, 2021, 6:37 a.m. UTC
These perfeclty good patches from 2014 weren't picked with no good
reason.

[1] https://lore.kernel.org/git/1398808178-3703-1-git-send-email-felipe.contreras@gmail.com/

Felipe Contreras (2):
  config: avoid yoda conditions
  add: avoid yoda condition

 builtin/add.c | 2 +-
 config.c      | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Junio C Hamano June 15, 2021, 1:45 a.m. UTC | #1
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.
Felipe Contreras June 15, 2021, 8:22 a.m. UTC | #2
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