CVS edit link for bblake

I want to begin contributing some of my code back to the community. In particular, this consists of small to medium sized modules that perform activities that are not in any existing projects but have definite value to Drupal users.

The first module is OpenID Hide, which provides a way for site administrators to remove the OpenID form from all but a hidden url. This is useful when site administrators may want to log in using OpenID but do not want the public to be able to do so.

In the future, I'd also like to contribute other modules, as well as help maintain some other modules that have been contributed by Phase2 Technology.

CommentFileSizeAuthor
#5 openid_hide.tgz1.61 KBbblake
#1 openid_hide.tgz7.2 KBbblake

Comments

bblake’s picture

StatusFileSize
new7.2 KB
bblake’s picture

Status: Postponed (maintainer needs more info) » Needs review
avpaderno’s picture

Issue tags: +Module review

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.

avpaderno’s picture

Status: Needs review » Needs work
  • 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. Hook implementation comments should be like the following one:
    /**
     * Implements hook_menu().
     */
    

    As reported in Documenting hook implementations:

    If the implementation of a hook is rather standard and does not require more explanation than the hook reference provides, a shorthand documentation form may be used in place of the full function documentation block described above:

    /**
     * Implements hook_help().
     */
    function blog_help($section) {
      // ...
    }
    

    This generates a link to the hook reference, reminds the developer that this is a hook implementation, and avoids having to document parameters and return values that are the same for every implementation of the hook. Optionally, you can add more information in a separate paragraph to describe the particular quirks of your hook implementation.

    In the case of hooks that have variables in the names, such as hook_form_FORM_ID_alter(), a slightly expanded syntax should be used:

    /**
     * Implements hook_form_FORM_ID_alter() for node_type_form().
     */
    function mymodule_form_node_type_form_alter(&$form, &$form_state) {
      // ...
    }
    

    This generates a link to the hook reference, as well as to the particular form that is being altered. Again, optionally you can add more information in a separate paragraph to describe the particular quirks of your hook implementation.

  4. Menu descriptions and titles, as well as schema descriptions, should not be passed to t().
  5. The module doesn't implement hook_uninstall(), or doesn't implement it to remove the Drupal variables it defines.
  6. /**
     * Implementation of hook_admin_settings()
     */
    
    

    Such hook doesn't exist.

  7. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how constant names should be written.
bblake’s picture

Status: Needs work » Needs review
StatusFileSize
new1.61 KB

Thanks for your comments. I've reviewed the comments and used the Coder module to clean some things up and have made all the changes required. Please give it another look.

zzolo’s picture

Component: Miscellaneous » miscellaneous
Status: Needs review » Postponed

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

  • The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
  • Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.
avpaderno’s picture

Issue summary: View changes
Status: Postponed » Closed (won't fix)

As per previous comment, I am setting this issue as Won't fix.
Since new users can now create full projects, applications have a different purpose and they are handled on a different issue queue. See Apply for permission to opt into security advisory coverage for more information.

avpaderno’s picture

Component: miscellaneous » new project application