[D7] Cache_node_object

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxKOsipenko1999032git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi, KOsipenko

Please moving from a master to a major version branch.

added description

Hey KOsipenko,

Looking at, cache_node_object.install.
I'm sure you know but be extra careful when storing serialized data as we don't know how large the node object is *could be. This comes back to the storage question of how big is too big... Maybe allowing the administrator to set the desired varchar size could prevent any accidental truncation(s).

cache_node_object.module.
I'm not sure how the community as a whole feels about echo vs print however I have only ever seen print used in other contrib modules. Not that this makes any real difference aside convention.

Line 158,
If your only using node data why not invoke node_save instead of entity_save?

Cheers,

My critique:
- I see you're using db_select() for your select queries which requires a lot more overhead than db_query(). For your select queries db_query() is several times faster than db_select(). If you need a limit query, db_query_range() is there.
- A little more detail would help the non-developers judge if this module meets their requirements. The details could be broken up into these sections: Overview, Features/Benefits, Requirements and Installation/Configuration.

I love this idea, it's a lot better than what I did in Drupal 5 in which I saved all my fields as a serialized string in the body field. If performed really well but was incompatible with the core search module. I feel this is how it should be done instead of having field_data and field_revision tables.

Good luck,
Gerald

I think the way to synchronize node object (function cache_node_object_synchronization) and menu (Synchronization of contents) is too hacky. (with the $_GET reset parameter that take the index down to 0)

Without looking at the code, i don't even know, how this is working.

I think you must implement an admin page that link to this menu callback, with an information on the number of node left to synchronise.

A must, will be that you implement a batch operation, instead of this hacky manual & hidden synchronisation.

For the purpose of this module, don't you think the drupal cache and compress page things, already accomplish (well, in another way, but..) what you module do ? i'm curious :)

Set to needs work because your menu callback need at least to be documented, and your README.TXT must be more detailled http://drupal.org/node/447604

Removing review bonus tag, you have not done all manual reviews, you just posted the output of an automated review tool. Make sure to read through the source code of the other projects, as requested on the review bonus page.

I removed the automated review from the issue summary.

@KOsipenko,

You are using some names of a functions like hooks e.g:

cache_node_object_update()
cache_node_object_load()

This names are actually hooks if your module specify content type so it will be better to rename them to something else e.g.

cache_node_object_object_update()
cache_node_object_object_load()

It is true that they are called only if your module defines a content type but when Drupal oriented developer reads your code can go wrong.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

removed automated review