[D7] Hide Nodetitle

Great project, ts145nera! Thank you for contributing it.

I only have a few simple things to point out, nothing major.
I recommend you submit it to http://ventral.org/pareview to carry out a more thorough review on your own.

* A 7.x-1.x-dev branch isn't necessary. Your latest code in 7.x-1.x can be made into a dev release using the project management release tools.
* The language on the project page could be reworked for greater readability. Also, Rules should be capitalized.
* A screenshot would help users quickly see what this does.

Great module !
I've been using auto node title before and this one doesn't force to give a pattern title, what can be very interesting in some situations.

works good on clean install.

I like the module.

I've tested the module and it works as expected.

Good work, clean code.

The module works as desired. Since Drupal believes in "collaboration over competition" so you could request the maintainers of auto_nodetitle to merge the code into their's.

Using http://groups.drupal.org/node/184389 and http://drupal.org/node/894256 as a template for this review.

README.txt

Check, file contains adequate information

project page

Please take a moment to make your project page follow tips for a great project page.

Master Branch

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.

Major coding standards / best practice issues

An automated review of your project using Coder has found 1 issue(s) with your code.
hide_nodetitle.module - Line 20: else statements should begin on a new line See 1st attachment.

An automated review of your project using the PAReview.sh at ventral.org found multiple issues with your code
See 2nd attachment or look here: http://ventral.org/print/192

License

License file detected by PAReview.sh in 7.x-1.x

access administration vs. administer site configuration

Irrelevant, there are no permissions.

files[] without classes or interfaces

3 lines in hide_nodetitle.info: 4, 5, 6.
It is only necessary to declare files[] if they declare a class or interface.

License

7.x-1.x Please remove the LICENSE.txt file. Drupal will add the appropriate version automatically during packaging so your repository should not include it.

Multiple Applications

No, this is it.

Idle Application

Nope, still active.

Code too short

This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project.
If we count lines among all the files (excluding .info and README.txt) you are VERY close, about 106 lines.

Duplicate module

Yes:
Specified as a simplification of the Automatic Nodetitles module: http://drupal.org/sandbox/nico059/1741784
Personally, I would rather use this module to achieve the desired effect because of its simple implementation and Rules integration.

This module should go to the repository (even if it has less than 120 lines) or be merged with automatic Nodetitle !
As said before by others the way it works is much more clean than automatic Nodetitles...

Just installed this module and the module works fine but the point raised by @carwin must also be taken into consideration.

Hi,

1. Concerning the warnings you should just remove the last 4 words " for the node form".
The comment line should be as follows:
* Implements hook_form_FORM_ID_alter().

2. Line 36. You use html inside a t() function. This should be done with placeholders as described on the API.
This:
t('Specify if node\'s title is visible or not during node insert. If <i>hide</i> is selected then title is <i>Undefined</i>. In this case you can use <a href="http://drupal.org/project/rules">Rules</a> project to generate it.'),

Should be:

t('Specify if node\'s title is visible or not during node insert. If <i>hide</i> is selected then title is <i>Undefined</i>. In this case you can use !rules project to generate it.', 
  array('!rules' => l(t('rules'), 'http://drupal.org/project/rules'))),

Concerning the italic <i> tag, it shouldn't be there neither (correct me if i'm wrong) as the t function is used to translate raw values only. Instead it is possible to concatenate multiple t functions and html tags.

'#description' => t('Specify if node\'s title is visible or not during node insert. If ').'<i>'.t('hide').'</i>'.t(' is selected then title is ').'<i>'.t('Undefined').'</i>'.t('. In this case you can use !rules project to generate it.'), 
  array('!rules' => l(t('rules'), 'http://drupal.org/project/rules'))),

3. line 8 & 9. Is this really necessary ?

define('HIDE_NODETITLE_DISABLED', 0);
define('HIDE_NODETITLE_ENABLED', 1);

4. Line 27 at .install. Is this function ever called ?
function hide_nodetitle_update_1()

5. You said that this module can be used with Rules. That would be great, but when I try to set an automatic node title using Rules I see that there is no way to select the option "Hide Node title enabled".

Improving this 5th point would make your module be more than 120 lines.

Good job !
Hope it will be contributed soon :-)

status change

Oh I've found a little annoying problem with the module.

When a content type has the "Hide title" checked is not possible anymore to add nodes of this content type to a menu checking "Provide a menu link" on the edit/create form.

Commenting line 24 solves the issue:
$form['title']['#type'] = 'value';

But then the title appears on the edit/create form.

Not sure if that can be considered as a bug, in any case an idea to solve this would be to hide the field using css.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.