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 _______________________________________________ greybus-dev mailing list greybus-dev@xxxxxxxxxxxxxxxx https://lists.linaro.org/mailman/listinfo/greybus-dev
|