Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> writes: > "Ritesh Harjani (IBM)" <ritesh.list@xxxxxxxxx> writes: > > Hello All, > > So I gave some thoughts about function naming and I guess the reason we > are ping ponging between the different namings is that I am not able to > properly articulate the reasoning behind, why we chose iomap_ifs_**. > > Here is my attempt to convince everyone.... > > In one of the previous versions of the patchsets, Christoph opposed the > idea of naming these functions with iop_** because he wanted iomap_ as a > prefix in all of these function names. Now that I gave more thought to it, > I too agree that we should have iomap_ as prefix in these APIs. Because > - fs/iomap/buffered-io.c follows that style for all other functions. > - It then also becomes easy in finding function names using ctags and > in doing grep or fuzzy searches. > > Now why "ifs" in the naming because we are abbrevating iomap_folio_state > as "ifs". And since we are passing ifs as an argument in these functions > and operating upon it, hence the naming of all of these functions should > go as iomap_ifs_**. > > Now if I am reading all of the emails correctly, none of the reviewers have > any strong objections with iomap_ifs_** naming style. Some of us just > started with nitpicking, but there are no strong objections, I feel. > Also I do think iomap_ifs_** naming is completely apt for these > functional changes. > > So if no one has any strong objections, could we please continue with > iomap_ifs_** naming itself. > > In case if someone does oppose strongly, I would humbly request to please > also convince the rest of the reviewers on why your function naming > should be chosen by giving proper reasoning (like above). > I can definitely help with making the required changes and testing them. > > Does this look good and sound fair for the function naming part? Hello All, Hope I didn't take too long to respond to my previous email. I had a discussion with Darrick and he convinced me with - - ifs_ naming style will be much shorter and hence preferred. - ifs_ already means iomap_folio_state_** v/s iomap_fs_** means iomap_iomap_folio_state... makes iomap_ifs_ naming awkward. - All of these functions are anyway local and static to fs/iomap/buffered-io.c file so it is ok even if we have a shorter names like ifs_alloc()/ifs_free() etc. Hence I have decided to go with ifs_ options for v10 which Darrick (including few others) agreed to here [1] [1]: https://lore.kernel.org/linux-xfs/87h6r8wyxa.fsf@xxxxxxx/T/#m7c061e634243f835ecddfb642bbfb091a9227ec7 -ritesh > > If everyone is convinced with iomap_ifs_** naming, then I will go ahead > and work on the rest of the review comments. > > Thanks a lot for all the great review! > -ritesh
