Thank you, Tim and Larry! I appreciate the suggestion and the feedback. I’m on board with using Conversion* as the class names.
Best, Ashvin On Fri, Mar 29, 2024 at 8:24 AM larry mccay <lmc...@apache.org> wrote: > I like the proposed renaming away from *Client's. > SourceReader and TargetWriter seem to imply that there are SourceWriter and > TargetReader though and otherwise it is kind of redundant. > At the same time, Source implies reading and Target implies writing, I > think. > > Given the ConversionController suggestion, what about ConversionSource and > ConversionTarget? > Maybe a little long but lines up well. > > Anyway, I don't have strong opinions here other than I am happy to see the > *Client's go. :) > > On Fri, Mar 29, 2024 at 8:40 AM Tim Brown <tim.brown...@gmail.com> wrote: > > > 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/ > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > > > > > > > >
