Re: [PATCH] greybus: Fix coding stye error

[Johan Hovold <johan@kernel.org>]

On Fri, Jan 27, 2017 at 04:24:47PM +0300, Dan Carpenter wrote:
> On Fri, Jan 27, 2017 at 02:08:06PM +0100, Johan Hovold wrote:
> > On Fri, Jan 27, 2017 at 03:47:21PM +0300, Dan Carpenter wrote:
> > > The name sucks is what I'm saying.  Please fix it eventually.
> > 
> > I disagree. The name uses a common prefix that reflects the object it is
> > working on. This should not be changed. As it is currently named, the
> > function is also self-documenting. Trying to abbreviate the name just to
> > meet a 80 col limit (when there are alternatives) is just silly.
> 
> gb_interface_request_mode_switch()
> 
> It's not self documenting because there are no verbs in that sentence.

"request" is a verb.

We're requesting that the remote interface initiates a mode switch
(something which it can refuse).

It's a function that is only a called a couple of times. No need to
rewrite it as gb_intf_rms() or such (even if gb_intf would be a
reasonable prefix).

You comments about missing verbs etc make me wonder how you parse the
name:

	gb_interface		- a common prefix, indicating object
				  being acted on
	request_mode_switch 	- describes what is done

we're not fetching anything as your "gb_get_mode_switch()" suggestion
indicates you read it as.

> I guess in JAVA it would be considered good style.
> 
> This is "Enterprise Quality" in the derogatory sense of the term.  The
> prefix makes everything too long and doesn't add any value.  We don't
> keep the 80 character limit because of small screens, we keep it to
> discourage this type of code.

And to keep indentation down.

The kernel already has similar function names such as
"usb_driver_set_configuration" or "pm_runtime_mark_last_busy" which
follow the same pattern (prefix - verb - object).

Johan