Hi Ashvin, I like your recommendation and reasoning for not including metadata in the name. I think SourceReader and TargetWriter are a good option. Including 'table' may make the names too long when also prefixing with a format name.
I agree on the need to change OneTableClient as well. ConversionController has my vote. -Tim On Thu, Mar 28, 2024 at 7:57 PM Ashvin A <ash...@apache.org> wrote: > Hello, > > As we’re considering the renaming of “clients,” I’d like to bring up the > OneTableClient as well. Given its role in overseeing the lifecycle of a > conversion request, it seems fitting to rename it in line with the source > and target clients. My suggestion would be to opt for either > ConversionController or TranslationController. Thoughts? > > Thanks, > Ashvin > > On Thu, Mar 28, 2024 at 4:38 PM Ashvin A <ash...@apache.org> wrote: > > > Hi Tim, > > > > The confusion regarding HTTP clients seems to stem from the term “Client” > > rather than “Source” and “Target”. Therefore, rather than substituting > > “Source” and “Target” with “Metadata”, would it be better to replace > > “Client” with "Reader" and "Writer" or "TableReader" and "TableWriter"? > The > > new base classes would then be “SourceReader” and “TargetWriter” or > > "SourceTableReader" and "TargetTableWriter". > > Additionally, if positional deletes are categorized as data, then the > term > > “MetadataReader” would not accurately reflect the nature of the source > > class. > > > > Thanks, > > Ashvin > > > > On Thu, Mar 28, 2024 at 11:39 AM Tim Brown <tim.brown...@gmail.com> > wrote: > > > >> Hi Everyone, > >> > >> As we are working through the renaming, I would like to suggest that we > >> rename the SourceClient and TargetClient interfaces to MetadataReader > and > >> MetadataWriter respectively. I think that some users and developers may > >> think of these as http clients. > >> > >> What does everyone think? Any other suggestions? > >> > >> Thanks, > >> Tim > >> > >> On Sat, Mar 23, 2024 at 11:24 PM Ashvin A <ash...@apache.org> wrote: > >> > >> > Hi Jesus, Tim, > >> > Thanks for the feedback. > >> > > >> > Tim, the ‘Internal’ prefix sounds good to me, it was my initial > thought > >> as > >> > well. It is simple and descriptive, and decouples the library’s name > >> from > >> > its functionality. > >> > My only worry is that it might lead to lengthy class names. However, > I’m > >> > willing to test it in a pull request and we can discuss it further if > >> > necessary. > >> > A shorter option is Inner. > >> > > >> > Best, > >> > Ashvin > >> > > >> > > >> > On Sat, Mar 23, 2024 at 6:08 AM Tim Brown <tim.brown...@gmail.com> > >> wrote: > >> > > >> > > One other option is to prefix with "Internal" or something similar > >> > instead > >> > > of the XT so it's clear it is our intermediate representation. > >> > > > >> > > -Tim > >> > > > >> > > On Fri, Mar 22, 2024 at 5:29 PM Jesus Camacho Rodriguez < > >> > > jcama...@apache.org> > >> > > wrote: > >> > > > >> > > > Thanks for starting this discussion, Ashvin! > >> > > > > >> > > > I think the proposal makes sense. Otherwise, we may find ourselves > >> > > needing > >> > > > to explicitly reference the classes using the namespace too often > >> for > >> > > > common names across table formats. > >> > > > > >> > > > -Jesús > >> > > > > >> > > > On Fri, Mar 22, 2024 at 12:09 PM Ashvin A <ash...@apache.org> > >> wrote: > >> > > > > >> > > > > Hello All, > >> > > > > > >> > > > > I wanted to discuss our class naming conventions, particularly > >> > > concerning > >> > > > > the use of prefixes. As we approach our first release, it's > >> crucial > >> > to > >> > > > > finalize a convention that enhances code readability without > >> > > compromising > >> > > > > on best practices. > >> > > > > > >> > > > > Classes such as DataFile and Schema often exist in all open > table > >> > > > formats. > >> > > > > Using the same name in XTable can lead to confusion. A short > >> prefix > >> > > like > >> > > > > 'XT' could distinguish these effectively. However, I am aware > that > >> > some > >> > > > > consider prefixing an anti-pattern and may have reservations > about > >> > this > >> > > > > approach. [1][2] > >> > > > > > >> > > > > For context, since XTable was previously OneTable, it has left > >> many > >> > > > classes > >> > > > > prefixed with 'One'. While we could continue this tradition, we > >> could > >> > > > adopt > >> > > > > a hybrid approach. For classes where ambiguity is high, we would > >> > adopt > >> > > > the > >> > > > > 'XT' prefix. In other cases, we would opt for non-prefixed > names, > >> > > > > maintaining simplicity and clarity. > >> > > > > > >> > > > > I believe this strategy offers a balanced solution, but your > >> input is > >> > > > > invaluable. Please share your thoughts and suggestions. > >> > > > > > >> > > > > Best, > >> > > > > Ashvin > >> > > > > > >> > > > > [1] https://www.yegor256.com/2020/03/03/prefixed-naming.html > >> > > > > [2] > >> > > > > >> https://www.nikolaposa.in.rs/blog/2019/01/06/better-naming-convention/ > >> > > > > > >> > > > > >> > > > >> > > >> > > >
