On Mon, May 21, 2018 at 12:13:06AM +0300, Or Gerlitz wrote: > On Sun, May 20, 2018 at 1:20 AM, Marcelo Ricardo Leitner > <marcelo.leitner@xxxxxxxxx> wrote: > > On Mon, May 14, 2018 at 05:27:14PM +0300, Vlad Buslov wrote: > >> Substitute calls to action insert function with calls to action insert > >> unique function that warns if insertion overwrites index in idr. > > > > I know this patch may be gone on V2, but a general comment, please use > > the function names themselves instead of a textualized version. I.e., > > s/action insert unique/tcf_idr_insert_unique/ > > disagree. While doing reviews I found out that if I ask the developer > to use higher > level / descriptive language and specifically to avoid putting > variable / function names in > patch titles and change logs, the quality gets ++ big time, vs if the > developer is allowed to say > > net/mlx5: Changed add_vovo_bobo() > > Added variable do_it_right to add_vovo_bobo(), now we are terribly good. In your example I agree that it is not helping and it is even allowing such empty changelog, just as in the section I highlighted, the descriptive language is also not helping IMHO. I had to read it 3 times to make sure I wasn't missing a modifier word when comparing the two functions and well, it's just saying "Substitute calls to foo function to bar function". I don't see how the textualized version helps in this case while, at least in this one, I would have visually recognized the function names way faster. Sounds like 2 bad examples for either approach. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
