On Mon, May 21, 2018 at 12:40:54AM +0300, Or Gerlitz wrote: > On Mon, May 21, 2018 at 12:33 AM, Marcelo Ricardo Leitner > <marcelo.leitner@xxxxxxxxx> wrote: > > 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. > > Properly written descriptive language is maybe harder to come up with > (specifically for those of us who are not native english speakers, like me) > but is more expressive and helpful for reviews and maintenance. Lets try > to add Vlad to come up with the right higher language and not ask him to > quote function and variable named. Alright. -- 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
