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/ > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > > > >
