fabian.ruiz [fabianruiz]

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

The description of your module made me think to the module Features (http://drupal.org/project/features). May you point out the differences between the modules, and eventually why you didn't open feature requests for the existing module?

1. See the coding standards to understand how a module should be written. In particular, see how the code should be formatted, and how the constants should be written.

2. 	$vec_items['admin/settings/briefcase'] = array(
   		'title' 			=> 'Briefcase',
   		'description' 		=> t('Briefcase configuration page'),
   

   Menu titles, and descriptions don't need to be passed through t() as that is already done from Drupal core code.

3. function _briefcase_goto($page)
   {
   	// Redirect to submodule "briefcase_cck"
   	if ( module_exists('briefcase_cck') )
   	{
   		if ($page == 'settings')
   		{
   			drupal_goto('admin/settings/briefcase/cck');
   		}
   		else if ($page == 'export')
   		{
   			drupal_goto('admin/content/briefcase/export/cck');
   		}
   	}
   	
   	// Redirect to submodule "briefcase_views"
   	if ( module_exists('briefcase_views') )
   	{
   		if ($page == 'settings')
   		{
   			drupal_goto('admin/settings/briefcase/views');
   		}
   		else if ($page == 'export')
   		{
   			drupal_goto('admin/content/briefcase/export/views');
   		}
   	}
   	
   	return '';
   }
   

   There is a much simply way to obtain that, and without write all that code. It's enough to correctly define the menu callbacks.
   Actually, that code is not even necessary, basing on how the sub-modules define their own menu callbacks.

4. if ( !eregi('^\.', $file_extension) )
   

   The code should use preg_match(), as that function has been deprecated in PHP 5.3.

5. drupal_set_message( t('Cannot create path: ').$directory, 'error');
   

   Use a placeholder.

6. function briefcase_cck_menu()
   {
   	$vec_items = array();
   
   	// Briefcase CCK configuration page
   	$vec_items['admin/settings/briefcase/cck'] = array(
   		'title' 			=> 'Briefcase CCK',
   		'description' 		=> t('Briefcase CCK configuration page'),
   		'page callback' 	=> 'drupal_get_form',
   		'page arguments' 	=> array('briefcase_cck_settings'),
       	'access arguments' 	=> array('administer site configuration'),
   		'file' 				=> 'includes/briefcase_cck.admin.inc',
       	'type' 				=> MENU_DEFAULT_LOCAL_TASK,
       	'weight' 			=> -10
   	);
   	
   	// Briefcase CCK export form page
   	$vec_items['admin/content/briefcase/export/cck'] = array(
   		'title' 			=> t('Export CCK'),
   		'page callback' 	=> 'drupal_get_form',
   		'page arguments' 	=> array('briefcase_cck_export_form'),
   		'access arguments' 	=> array('access content'),
   		'file' 				=> 'includes/briefcase_cck.admin.inc',
   		'type' 				=> MENU_LOCAL_TASK,
   		'weight' 			=> -9
   	);
   	
   	// Briefcase CCK import form page
   	$vec_items['admin/content/briefcase/import/cck'] = array(
   		'title' 			=> t('Import CCK'),
   		'page callback' 	=> 'drupal_get_form',
   		'page arguments' 	=> array('briefcase_cck_import_form'),
   		'access arguments' 	=> array('access content'),
   		'file' 				=> 'includes/briefcase_cck.admin.inc',
   		'type' 				=> MENU_LOCAL_TASK,
   		'weight' 			=> -8
   	);
   
   	return $vec_items;
   }
   

   The menu definition is not correct; there is a MENU_LOCAL_TASK without a MENU_DEFAULT_LOCAL_TASK.

7. function briefcase_views_menu()
   {
   	$vec_items = array();
   
   	// Briefcase Views configuration page
   	$vec_items['admin/settings/briefcase/views'] = array(
   		'title' 			=> 'Briefcase Views',
   		'description' 		=> t('Briefcase Views configuration page'),
   		'page callback' 	=> 'drupal_get_form',
   		'page arguments' 	=> array('briefcase_views_settings'),
       	'access arguments' 	=> array('administer site configuration'),
   		'file' 				=> 'includes/briefcase_views.admin.inc',
       	'type'				=> MENU_LOCAL_TASK
   	);
   	
   	// Briefcase Views export form page
   	$vec_items['admin/content/briefcase/export/views'] = array(
   		'title' 			=> t('Export Views'),
   		'page callback' 	=> 'drupal_get_form',
   		'page arguments' 	=> array('briefcase_views_export_form'),
   		'access arguments' 	=> array('access content'),
   		'file' 				=> 'includes/briefcase_views.admin.inc',
   		'type' 				=> MENU_LOCAL_TASK,
   		'weight' 			=> -9
   	);
   	
   	// Briefcase Views import form page
   	$vec_items['admin/content/briefcase/import/views'] = array(
   		'title' 			=> t('Import Views'),
   		'page callback' 	=> 'drupal_get_form',
   		'page arguments' 	=> array('briefcase_views_import_form'),
   		'access arguments' 	=> array('access content'),
   		'file' 				=> 'includes/briefcase_views.admin.inc',
   		'type' 				=> MENU_LOCAL_TASK,
   		'weight' 			=> -8
   	);
   
   	return $vec_items;
   }
   

   The menu definition is not correct, for the same reason I reported before. Then, what would happen if the user enables one sub-module, but not the other? There would be again one MENU_LOCAL_TASK without a MENU_DEFAULT_LOCAL_TASK.
   I would rather use a single module, and define the menu in that module.

8. 		'#title' 			=> t('CCK - Export Path'),
   

   Strings used in the user interface as title of items should be in sentence case.

Thank you for your contribution! I am going to update your account.
These 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 more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.