Exposes all Boxes as Tokens, as well as providing help information to show what tokens are available.

Project Page: http://drupal.org/sandbox/bnewtonius/1627914
Git: git clone http://git.drupal.org/sandbox/bnewtonius/1627914.git token_boxes
Version: Drupal 6.x-1.0-beta1

CommentFileSizeAuthor
#15 Screenshot at 2012-08-13 15:59:41.png117.09 KBdrupaldrop

Comments

sirkitree’s picture

sirkitree commented
  1. You have a drupal 6 version in master, which is fine, but you will probably want to create a 6.x-1.x branch for creating releases.
  2. Simple and quite useful!

bnewtonius++

q0rban’s picture

q0rban commented

You should use token placeholders in your t() usage, instead of concatenating strings. Otherwise, no one can actually translate the line, because it will be different for every delta. See this page for more details: http://drupal.org/node/322729 Otherwise, looks good! +1 :)

q0rban’s picture

q0rban commented

Whoops, meant this page: http://drupal.org/node/322732

muneer1st’s picture

muneer1st commented
Status: Needs review » Needs work

You should Remove "version" from the info file, it will be added by drupal.org later on.

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

bnewtonius’s picture

bnewtonius commented

Thanks for the advice! I'll get the version branch sorted out by this weekend, and hopefully the t() function.

bnewtonius’s picture

bnewtonius commented

I've changed the .info file and the version branch to both be: 6.x-1.0-beta1

bnewtonius’s picture

bnewtonius commented
Status: Needs work » Needs review

I believe I've resolved all issues raised in this comment thread. t() function is now parameterized. I also did some cleanup to remove the redundant "token_boxes" in front of the token so that you don't have to type [token boxes token_boxes_foo_bar] and can now just use the shorter [token boxes foo_bar]. Lastly, I've added a bit more documentation on usage.

dev001’s picture

dev001 commented
Status: Needs review » Needs work

Manual review, fix these issues:
1. Definition doesn't exist for function boxes_load() in module file.
2. Readme file is not there in the git 6.x-1.x.
3. Hook implementation should be documented as such: http://drupal.org/node/1354#hookimpl

bnewtonius’s picture

bnewtonius commented

Thank you loginradius for reviewing my module. I've removed the 6.x-1.x branch from git, it's not needed yet. I also changed the hook documentation to match the comment formatting convention.

I'm not sure I understand the first one though. boxes_load is defined in the boxes module, which is listed as a dependency in the .info file. Is there something else I need to do to document this?

Thanks!

bnewtonius’s picture

bnewtonius commented
Status: Needs work » Needs review
AtomicCharles’s picture

AtomicCharles commented

I like the simplicity of the module. It seems efficient and straightforward

The only things I would add would be to: (1) Add a README file and (2) Remove the version data from the .info file.

sreynen’s picture

sreynen commented

I've removed the 6.x-1.x branch from git, it's not needed yet.

I think you have this mixed up. A 6.x-1.x branch is the first thing you need. That's where you'll commit all updates between releases. When you're ready to do a release, you'll add a tag (not a branch) from the 6.x-1.x branch with a release number, e.g. 6.x-1.0-beta1.

bnewtonius’s picture

bnewtonius commented

Thanks sreynen. You're right and I see where I misread the documentation on branch and tag conventions. I've removed the beta1 branch. I also created a new branch from master called 6.x-1.x, moved my code into it, and committed it. I wanted the branch to be clean from master and not back from the beta1 branch.

Do I need to tag this branch now? I don't think there are any outstanding issues.

bnewtonius’s picture

bnewtonius commented

I've also removed version from the .info file as well

drupaldrop’s picture

drupaldrop commented
StatusFileSize
new Screenshot at 2012-08-13 15:59:41.png117.09 KB
  • There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
  • On enabling a module i didn't found a token filter under input format. Please see the attached screenshot
bnewtonius’s picture

bnewtonius commented

Thanks for the feedback drupaldrop.

I read steps 6 and 7 in that guide, and it lists deleting the master branch as optional. I will double check to make sure that the 6.x branch is the default as well (thought I had already set that).

To see token filters under input formats, you'll need to install and enable the token_filters module: http://drupal.org/project/token_filter (as described in the "usage" and "examples of usage" sections on the projects landing page).

EDIT: I found the right option to set the default branch. It should now be correct. Thanks again for pointing that out.

ro-no-lo’s picture

ro-no-lo commented

I never heared of the boxes project, but I looked into your code anyway. 40 lines of code and no visual problems or violation of the drupal coding standard.

I would suggest to write the usage from your project page also into the README.txt, because there is the need of help.

sreynen’s picture

sreynen commented
Status: Needs review » Needs work

ronan.orb, make sure to change the status to match your comment. Sounds like this should be "needs work" now.

ro-no-lo’s picture

ro-no-lo commented
Status: Needs work » Needs review

Nah, it's a nice to have. Not a requirement.

bnewtonius’s picture

bnewtonius commented
Status: Reviewed & tested by the community » Needs review

Not a bad idea though. I'll try to get this added soon.

EDIT: Documentation added to the README.txt file.

sirkitree’s picture

sirkitree commented
Status: Needs review » Reviewed & tested by the community

I think this is really ready to go guys.

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Status: Needs review » Reviewed & tested by the community

agreed, this is ready for promotion.

sreynen’s picture

sreynen commented
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, bnewtonius!

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:

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 reviewers as well.

bnewtonius’s picture

bnewtonius commented

Thanks sreynen!

Status: Fixed » Closed (fixed)

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

bnewtonius’s picture

bnewtonius commented
Title: Token Boxes » Token Boxes Project Application
bnewtonius’s picture

bnewtonius commented
Issue summary: View changes

updating the version number