igor.ro [igorro]

Easy Authcache is module that makes authcache implementation easy,faster and makes it comportable with drupal community modules.

Advantages of module differs it from Authcache:

1. It makes unnessary for developer to write client side callbacks for authcache module (js functions).
2. Makes server side callback as hook (drupal style).
3. Includes all modules.
4. Handle js files and settings, needed for retrived with ajax html, on final page.
5. Handle url context dependent code.
6. Provide api to handle theme functions, you need, with authcache.

Disadvantages of module differs it from Authcache:

1. Makes bootstrap FULL on authcache ajax callback. That can make slowly then authcache manual implementation.
2. Can lose some js Drupal.settings after authcache ajax result.
3. Higher, then for manual authcache implementation can be, load on client side (js code).

Documentation with description how easy authcache works.

Hello, and thank you for applying for a CVS account. I am adding the review tags, and some volunteers will review the code, pointing out what it needs to be changed.

Did you submit a feature request for each of the features you think missing in the existing project, and that you implemented in the proposed module?

Hello thanks for your work.
No I do not post feature request for all features I did with this module, because modules have different goals.
Authcache allow to create fast ajax requests, by writing custom code, and because of skipping bootstrap steps. For example you can load just SESSION_INiT bootstrap step and then include modules we need. Easy authcache allways make FULL bootstrap and it's goal is to implement ajax getting very fast, to meet deadline. In other words Authcache is lowlevel and Easy authcache is high level tools.
But you are right, as soon as I have posted module for cvs account I'll ask maintainer of Authcache module to review code.
I have used this module on 3 projects and my colleges on 2 other projects. And because they are happy, I think it is good enought to be in community.
Thanks.

Thank you for your reply.

• This is a partial review.
• The points reported in this review are not in order of importance / relevance.
• Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
• Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.

1. License files cannot be committed in Drupal.org repository. Projects committed in Drupal.org repository have the same license used by Drupal.
2. The version line needs to be removed from the .info file.
3. The module doesn't implement hook_uninstall(), or doesn't implement it to remove the Drupal variables it defines.
4. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code should be formatted; how Drupal variables, global variables, constants, and functions defined from the module should be named.
5. The code depends from a PHP5 function, but the module doesn't declare its dependency from that version of PHP.

Thanks.
Going to fix asap.

I have fixed issues.
Hope code is good ehought for detailed core reivew.

Thanks

Anyone, I need community review, please.
Thanks.

Hello Igor,

I'm sorry; you've waited a long time and have clearly put effort into this code. It is definitely ready to be shared as a real project with issue queues. Thank you for your contribution and patience and good luck with your Drupal coding!

^{benjamin, agaric}

Hi @igor.ro, thanks for the application and patience. Congratulations, you have been approved. Great work on your code and we look forward to your contributions to the Drupal community!

The following points are recommendations for when you do put your code on drupal.org and develop it further; these are not requirements, simply suggested improvements based on my experience and knowledge of the existing Drupal community. Also, please read further about CVS access and resources.

1. You are using 4 spaces, when you should use 2. You could do a little better on the coding standards, but great for a first start. The Coder module can help a lot, and reading all the pages in and under http://drupal.org/coding-standards
2. As @kiamlaluno point out above, you need to put PHP5 as a minimum for your module in your .info file.
3. It is unusual to have an underscore leading a directory name.

Please read the following resources to make sure you know how to use CVS and the specifics to the Drupal CVS infrastructure, as well as how to be a good module maintainer on Drupal.org. The Drupal community is very large and dynamic; we welcome you as a module maintainer and hope that you embrace and challenge the Drupal community and continue to contribute.

• CVS reference: http://drupal.org/handbook/cvs/quickstart
• Maintaining modules: http://drupal.org/node/7765

Thanks to the following people who helped review this application, it is very appreciated:

• @kiamlaluno
• @mlncn

--
Note: Please be patient with the CVS application process. It is all done by volunteers. Our goal is not to be arbitrarily slow or meticulous. Our goal is to get you CVS access and ensure that you are and will become a more responsible Drupal contributor. For a quick reference on what I look for in a code review, please see this article, or read the handbook page on how to review for reference.

I was going to report this points, but my connection had problems.

1. 
   /**
    * Implementation hook_install()
    */
   function easy_authcache_install() {
    variable_set('easy_authcache_single_request', TRUE);
    variable_set('easy_authcache_max_age', 3600);
   }
   

   Drupal variables are not initialized in hook_install(); variable_get() accepts a parameter that is used as default value when the variable has not been set.

2. Hook comments should like the following:

   /**
    * Implements hook_theme_registry_alter().
    */
   

3. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see the indentation that should be used for the code; how Drupal variables, PHP variables, and functions defined from the module should be named.
4. The module uses a PHP5 function, but it doesn't declare its dependency from that PHP version. Differently from Drupal 7, Drupal 6 is still compatible with PHP4.
5. Drupal has a function that can be used insted of json_encode().
   
6. easy_authcache_ui.admin.inc contains just a function; it would be better to merge it with the module file.
7. The JavaScript file should use Drupal behaviors.

Thanks.
This is good present on New Year.
Merry Christmas.

Thanks zzolo for review.
All your issues are absolutly right. Thanks. Will fix asap.

Thanks kiamlaluno for your review.
Especially for your it's specification.
I had reasons to do that way.
For example.
1. About variables on install, it makes me useful to remember what variables I had to remove on uninstall. But I've clear minds that it makes us to store default values in DB and potentially makes it slow.
2,3. You are absolutely right about comments and function calls. Thanks.
But there are a few functions I can not change name because they called according authcache callback function name agreement.
And that agreement differs from drupal ones.
4. You are right.
5. I'll check. But as I remember now authcache used js code to convert objects to json and that code is crappy, it makes
. So I had problems with default json_encode(); and had to write my own.
6. I thinking it is good practice to make files as small as we can and include on demand. But sure I'll check your idea ;)
7. This is principal point for easy authcache module to do not use behaviours, because we need to get dynamic regions asap and only once - on document ready. Behaviors will make run it on every ajax return ;) So this is not clear drupal way but it is necessary do be that way.

Thanks.

Hey, to check out how to use Drupal.behaviors correctly, look at the "Drupal.behaviors practical example" section on http://drupal.org/node/205296

Module released thanks to every one Easy Authcache

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

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade .

I am giving credits to the users who participated in this issue.

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