Beanstag

Is there a reason you could not contribute to the Nodewords module to support including the language prefix in the URL? Generally it's better to improve an existing module than to create an all-new one.

Note that the Nodewords version for Drupal 7 is actually out now: http://drupal.org/project/metatag. Meta tags quick also supports meta tags by path/alias too. Maybe you should give it another look?

Looks like this needs a response to #3.

@ykyuen has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

If you want a review of your application, please switch it to need review. Also, you have to list the differences between your project and existing projects, if it is just simpler and more user friendly, you should maybe help the existing modules to become simpler and more user friendly?

I think all the reviewers are right, nodewords/metatags has already running on huge websites, and i don't think so it not so complicated in any terms of uses, i will suggest you to if your are not happy with nodewords/metatags module then post an issue with feature request and try to add some patches to it.

There is no problem to keep a project in a sandbox, nobody would delete it.

Hi, metatag D7 is still not working for panels using all tokens, also metatag is not possible to get different meta tags based on the specific url or alias. Beanstag is doing that. I think there is now space for a real beanstag project, while somehow metatag is not bringing the functionality of D6 yet. Sorry if this is incorrect statement in your view.
Greetings, Martijn

metatag is great when it works, but when it doesn't, it can be a headache. Beanstag saved me on two previous projects.

Given #16 comment - surely this project should be released as a full projec, no?

Hi ykyuen,
thank you for your light-n-easy solution.

1) Git. You should create another branch called 7.x-1.x and set it to default. See http://drupal.org/node/1127732
2) Manual code review. beanstag.info: "Beansbox" is bad package name, may be it's better to rename package to "SEO" or something else?
3) UI. I have an idea how to edit page title and meta tags. You can display them on the bottom of the page where user can edit them and click "Save".

For #2, see http://drupal.org/node/542202#package "In general, this property should only be used by large multi-module packages, or by modules meant to extend these packages, such as Fields, Views, Commerce, Organic Groups, and the like. All other modules should leave this blank."

Both Node Words and Meta Tags modules use the package name "Meta tags" so I think that's as much of an established package as we have here. You should either use an established package or nothing. Creating a new package with only one module in it makes package organization less useful for site administrators.

Can we have another review so this can finally be released? After all the hard work put in I think that would be good :)

kingfisher64, anyone can do reviews, so you don't have to wait on someone else. You can do it yourself. If you'd like to see this move forward, you can find instructions for how to do reviews here: http://drupal.org/node/894256

sreynen - right thanks. I didn't know that. So much drupal.org documentation to get through.

Thank you.

Automatic Review:
There are a lot of issues that you need to fix (http://ventral.org/pareview/httpgitdrupalorgsandboxbeansboxchrispang1189...)

Manual Review:

• The name of your admin.inc file is confusing. I thought it contained the main admin form, but this one is in the .module file. Maybe you can also move this form to this include?
• If I understand correctly, you have to press update beanstag to create/update the beanstag for the current path alias of the page. Maybe you can implement some functionality to make this an automated process through a cron hook?

After my review, probably some work needs to be done to make it at least pass the PAReview process.

After my review, probably some work needs to be done to make it at least pass the PAReview process.

A few minor things I noticed:

beanstag.install: Your update method name is beanstag_update_7000. My understanding from the documentation is that you should probably have beanstag_update_7100 since your current branch is 7.x-1.x.

beanstag.admin: In beanstag_admin_form(), just from a readability standpoint, I think it might be better to initialize the $form_data to just array(), and then add the associative keys as your first line of code if the array is empty. Something like this:

function beanstag_admin_form($form, &$form_state, $form_data = array()) {
  if(empty($form_data)) {
    $form_data['path_alias'] = '';
    $form_data['page_title'] = '';
    $form_data['meta_keywords'] = '';
    $form_data['meta_description'] = '';
    $form_data['meta_robots'] = '';
    $form_data['meta_canonical'] = '';
    $form_data['id'] = NULL;
  }

  // Then the rest of your code here.
}

It's probably just my personal preference, but something to consider.

In general, I would suggest adding a little more documentation. Specifically, your comment headers are missing the @param and @return directives. See Doxygen and comment formatting conventions for more info.

Other than these minor things, the code looks really clean. I have changed the status to 'needs work' for the previous items.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Automated review

All clean, no issues.

Manual review

1. Uninstall cleans up properly.
2. On my first Beanstag creation right after enabling the module and opening the creation form:
   

   Warning: Missing argument 3 for beanstag_admin_form() in beanstag_admin_form() (line 93 of /Users/ain/NetBeansProjects/Drupal7/sites/all/modules/beanstag/beanstag.admin.inc).

3. I was not able to get meta tags for a page at ?q=node/2 on a bare Drupal 7 installation with Token module disabled, Override and Robots checked in for configuration. If I did do something wrong, I should have help (or errors) to guide me through and provide the exact requirements. It could also be over the issue described in last point.
4. Security testing not possible due to the issues.

Recommendations

1. Validate path alias so that the meta tags can only be added for aliases that actually exist.

Don't forget to add the "PAReview: review bonus" tag as indicated in #1410826: [META] Review bonus, otherwise you won't show up on my high priority list.

Hello Ykyuen!
Cool looking module! Here is my manual review:

Misc
- your files permissions are set to 775 you should change them to 644

Project Application Issue
- no git repo link in issue

Project Page
- Add the same link from your README.txt to your project page:
"For more information, visit http://eureka.ykyuen.info/tag/beanstag/"

README.txt
- Consider including the same sections on your project page in yur README.txt: Overview, Features, Installation, How to use, Known problems

beanstag.admin.inc
- line 68 you have a duplicate condition:

$prev_path = (isset($_SESSION['last_request_page']) && isset($_SESSION['last_request_page'])) ? $_SESSION['last_request_page'] : '';

shouldn't this be?

$prev_path = (isset($_SESSION['last_request_page'])) ? $_SESSION['last_request_page'] : '';

Testing
- manual test cases passed!

Hi ykyuen,

Automated review passed without issues.

Manual Review:
Code is very clean and follows Drupal's standards and best practices. All manual test cases passed, and module uninstalls correctly.

Recommendations:
On the project page description, I would list the benefits of Beanstag over other similar modules in addition to the fact that it's easier to use. Perhaps mention that it works for panels using all tokens, and that it is possible have different meta tags based on a specific URL or alias, unlike Meta tags D7.

Cheers

I would also add, the metatags module sometimes has issues, so this module can be a temporary fix while they patch issues. I use the metatags module, but it doesn't always play nice with the front page for some themes, so beanstag (which I would have named 'Simpletags', by the way) helps me bridge this.

manual review:

1. beanstag_admin_form(): why do you have to use the raw $form_state['input'] here and not $form_state['values']? You should always use $form_state['values'] where possible.
2. beanstag_admin_form_submit(): "t('Your BeansTag (id = %id) is updated successfully.', array('%id' => check_plain($form_state['values']['id'])));": the "%" placeholder will already run check_plain() for you, so no need for that. Make sure to read the documentation of t() again.
3. beanstag_bulk_delete_form(): use check_url() to sanitize URLs instead of check_plain().
4. beanstag_load(): doc block is wrong, this is not a node hook.
5. beanstag_exit(): the user module is loaded anyway, so no need for the extra module_load_include().
6. beanstag_request_path_to_arg(): why is that needed? $_SESSION['prev_path'] is not read anywhere? Please add more comments or remove it.
7. beanstag_request_path_load(): that function is never used? Same for beanstag_request_path_to_arg()?
8. beanstag_set_meta(): no need to run check_plain on the #attributes as the theme function will run drupal_attributes anyway for you. And double escaping is bad.

So that are some major issues, but no critical application blockers and I think this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to stBorchert as he might have time to take a final look at this.

No objections for more than a week, so ...

Thanks for your contribution, ykyuen!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Oh, thanks Klaus for taking this ...