c6a78ca8bacef1fae86eefa46e619d08f69e9cda - This kind of logic is sloppy. If at all possible, we should ensure that the data is _always_ serialized or unserialized by the time it gets into this method. (I'd prefer unserialized).

so maybe it should be on repo cache class

Comments

sdboyer’s picture

sdboyer commented

Yep. The cache is conceptually responsible for bringing the repository out of the database and preparing it for use, so it's the appropriate place to unserialize.

marvil07’s picture

marvil07 commented
Version: 6.x-1.0-rc2 » 6.x-2.x-dev
marvil07’s picture

marvil07 commented
Priority: Normal » Minor

I'm reviewing this now, but ..

I suggested to make that on the repo cache class, but repo cache class instances the repo class.
In general, we are not using cache classes from the class itself, so in that way we avoid circular references.

And in this case in specific, repo cache class instances repo class, so repo cache class __calls__ repo::build() and then, we want to have that unserializing there :-/

suggestions? or I suppose is better to mark this as by design(or think another way to change is_string() to something else)

marvil07’s picture

marvil07 commented

Now that we use a VersioncontrolEntityController, that relies on dbtng, so IIRC the serialized thing is done at that level, and we do not have to deal with it.

@sdboyer, please correct me if I'm wrong, if not, we can close this issue.

sdboyer’s picture

sdboyer commented
Status: Active » Fixed

Yep, this is all taken care of nicely now with the entity controller system. Actually, it's settling on VersioncontrolEntity::build() that make it all unified and set.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.