Ok summarizing responses from kanak and kishore (1) Should we rename PropertyType to HelixRecordType?
[KG] I don't like either:-). Does not really answer your question, but here is the analogy - Each Helix propertyType maps to a table in a database. HelixRecord is a row in the table, where HelixKey, HelixValue, Version are the columns in the table. [kanak] Shouldn't this be internal? [sandeep] If its internal I can move it out of API, not sure if a user ever wants to read the value held in a record to identify the type. If we expect to expose this then we need a name. If the table contains records then maybe a better name is HelixRecordContainer? (3) ZNRecordDelta for now is HelixValueDelta but will eventually move out of API [kanak] We need to think about this a little. The easiest way to implement commands is to maintain a delta internally. Obviously it's not an API because it's an implementation detail, but what is the most common pattern for something like this? [sandeep] I will put some thought into how we can keep this internal, I have some ideas but want to think them through before I throw something out there for consideration. (4) Should we rename ConfigScope to HelixConfigScope [KG] ConfigScope should not exist, we should have only Scope and should be used across all properties. [kanak] There's already something called HelixConfigScope. Realistically I think that we should use Scope for API and keep ConfigScope/HelixConfigScope internal. [sandeep] If ConfigScope should not exist then I'd rather get rid of it than keep it internal. I believe what KG is saying (and correct me if I am wrong) is each config can then carry Scope on it. 6) Should PropertyPathConfig be renamed to HelixRecordPathConfig [KG]This should definitely be in spi and not exposed to user. [kanak]Again, my intuition is that this should remain internal. Will this cause problems? [sandeep] I will keep it internal but need to figure how deeply referenced/embedded this is, I don't have the code right now in front of me so will figure a way to keep this internal. That would mean we need to break the dependency on this class from classes in API. I will take a look when I get back to the code. Thanks for your responses guys Sandeep On Tue, Mar 4, 2014 at 11:31 AM, Kanak Biscuitwala <[email protected]> wrote: > > Thanks for the update and your help. Responses inline. > > ---------------------------------------- >> Date: Tue, 4 Mar 2014 10:51:40 -0800 >> Subject: API rework: Questions on model classes moving to api from core >> From: [email protected] >> To: [email protected] >> >> Hey guys, >> >> I have been working on the list of TODO I sent before (copied here for >> convenience) >> >> (a) the HelixProperty realignment as outlined in (1) above >> (b) reworking some objects from config into model (see email exchange >> for API rework summary 03-01-2014 thread) >> (c) move more models into API, cleanup as necessary >> (d) start on command objects >> >> I have been chipping away at (a) and (b) but have a few questions to ask >> >> (1) Should we rename PropertyType to HelixRecordType? >> > > Shouldn't this be internal? > >> (2) ZNRecord like we discussed is now HelixValue >> >> (3) ZNRecordDelta for now is HelixValueDelta but will eventually move out of >> API > > We need to think about this a little. The easiest way to implement commands > is to maintain a delta internally. Obviously it's not an API because it's an > implementation detail, but what is the most common pattern for something like > this? > >> >> (4) Should we rename ConfigScope to HelixConfigScope >> > > There's already something called HelixConfigScope. Realistically I think that > we should use Scope for API and keep ConfigScope/HelixConfigScope internal. > >> (5) HelixProperty is renamed to HelixRecord which has HelixKey and HelixValue >> >> (6) Should PropertyPathConfig be renamed to HelixRecordPathConfig > > Again, my intuition is that this should remain internal. Will this cause > problems? > >> >> Let me know, I have been moving classes from core/model into api/model >> and dropping off the interface classes and fixing the compile issues. >> The model package has references to a lot of classes in core so I am >> moving constants into the api and adding reference from core to api. >> >> I will keep you guys posted when I get through (a) and (b) but would >> certainly appreciate input on the above questions. >> >> Thanks, >> >> Sandeep >
