Content Directory (Admin View)

Hey SleejR,

Thanks for the quick reply!

I am in the middle of making all the changes.

This is the first time I am trying to make one of my modules public, so I am very new to all of this.

I was wondering if there were a few things you could clear up as the documentation doesn't quite spell things out very well:

Please also add to link to your git clone ie : git clone http://git.drupal.org/sandbox/d.cochran/1549710.git content_directory__admin_view_

I posted this link to my git clone (is it not valid?): git clone --recursive --branch 6.x-1.x d.cochran@git.drupal.org:sandbox/d.cochran/1549710.git content_directory

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732

Under "Setting up this repository for the first time:" which is what you follow when creating your modules, it doesn't have you create a "README.txt" file, in fact it has you create a .info file in your master branch. If the "README.txt" is the only thing you are supposed to have, why is it having you create a .info file? Please let me know if both are supposed to exist, I'd say the "Setting up this repository for the first time" documentation needs to be updated ;^)

To further this, there is nothing under the version control that says you need to be working in a "branch". There are instructions on how to switch, but nothing says you "should" be developing under a version specific branch. The "Setting up this repository for the first time" should really include creating the "README" file, and it should include the setting up the branch, as these are both requirements that are neglected.

Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

Nice! I didn't know of http://ventral.org/pareview! I even checked out many reviews and never ran across it. I only knew of "Coder" which didn't seem to scan the .tpl.php files. It would be nice to have these two sources posted on the version control page or the "Apply for Full Access" page as well. Seeing as they are standards, and arn't mentioned at all. I found out about "Coder" after reviewing other peoples modules and I didn't even see http://ventral.org/pareview.

*EDIT* I did find reference to this two links under "Tips for ensuring a smooth review". I must have missed that link from all the other tabs I had open. Although, I wouldn't consider these items as "tips" seeing as they are standards. They should really be a part of the process...

Once again, thanks!

I will make the changes and repost.

~dc

Hey ankitchauhan,

Thanks for trying to review it, this is the git repository I was using, please let me know if it is invalid:

git clone --recursive --branch 6.x-1.x d.cochran@git.drupal.org:sandbox/d.cochran/1549710.git content_directory

I am currently making huge changes to this module...

Changing status to "needs work" until I am ready to have it checked again.

Adding: http://git.drupal.org:sandbox/d.cochran/1549710.git

I made huge updates to this module. A couple key points:

• This module no longer uses a .tpl.php file to build forms
• All forms are now built within form api
• All forms are built within their own .inc file
• This module no longer overwrites the current admin node display
• This module adds to your content management menu and creates its own page (admin/content/directory)

I have checked coder and pareview.

Pareview gives me a lot of spacing and indentation errors that I can't figure out why.

For example, if someone could tell me what is wrong with this:

ln16:  if (!$_POST['selected'] || !$_POST['operation']) :
ln17:    drupal_set_message(t('Nothing was selected!'), 'error');
ln18:    return FALSE;
ln19:  endif;

Pareview tells me:
16 | ERROR | An operator statement must be followed by a single space
18 | ERROR | Line indented incorrectly; expected 2 spaces, found 4

I don't think that adding a space after the "!" is correct.
The "return FALSE" belongs inside the "if" statement.

All of the spacing errors seem to be like this.

Could someone shed some light please?

Many thanks!

Similar?
I couldn't find a similar module that would let me browse through my content and taxonomy as if it were in a directory structure. I also tried creating a view several times and simply failed at it. I couldn't even get close, lol, maybe I just suck at views. I could obviously pull the content, but I was having issues with only getting the content for the current path and such. Anywho, that is another story.

Image License
Some are mine, others are creative commons from iconfinder.com

Alternative Syntax
This is something I will need to fix.

Structure
For such a small module, I didn't really see the need. This also seems like a personal preference. There are a ton of small modules out there that do not create a directory for the css.

Empty PHP Variable
I will take a look at it.

Unsecure Code and None Drupal Way
I did read the Writing Secure Code page before programming. In fact, I followed the examples posted on the page:

Instead, use proper argument substitution with db_query:

 db_query("SELECT foo FROM {table} t WHERE t.name = '%s' ", $_GET['user']); 

I also used the $_GET variable to mimic what drupal was doing with the "destination" variable. My path for the directories was pretty much the same thing as drupal's path to record the "destination".

Formatting
For some reason, I can not see the indenting, see post #8.

Overall
If there is no need for this module, we can scrap it. It was my first attempt and I learned a lot with it.

Thanks for taking the time to review it.

~dc