[D7] SN QUICK FIELD DELETE

Good morning gaurav.pahuja,

please include:

• detailed information on how to clone your module, not just a git link
• Do some reviews of other modules and include them here. It will help your application to be reviewed quicker
• The module itself looks too short to be reviewed, check the Project application checklist: https://drupal.org/node/1587704

Apart from that, the module itself has some things to correct to follow the coding standars:

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AND 1 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
4 | WARNING | Line exceeds 80 characters; contains 145 characters
20 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/sn_quick_field_delete.module
--------------------------------------------------------------------------------
FOUND 25 ERROR(S) AND 1 WARNING(S) AFFECTING 17 LINE(S)
--------------------------------------------------------------------------------
18 | WARNING | Avoid backslash escaping in translatable strings when possible,
| | use "" quotes instead
23 | ERROR | Missing short description in function doc comment
25 | ERROR | There must be an empty line before the parameter block
25 | ERROR | Expected a valid @param data type, but found type
25 | ERROR | Missing comment for param "$form" at position 1
26 | ERROR | Expected a valid @param data type, but found type
26 | ERROR | Missing comment for param "$form_state" at position 2
27 | ERROR | Expected a valid @param data type, but found type
27 | ERROR | Missing comment for param "$form_id" at position 3
35 | ERROR | Array indentation error, expected 6 spaces but found 8
36 | ERROR | Array closing indentation error, expected 4 spaces but found 6
41 | ERROR | Array indentation error, expected 6 spaces but found 8
42 | ERROR | Array closing indentation error, expected 4 spaces but found 6
56 | ERROR | Missing short description in function doc comment
58 | ERROR | There must be an empty line before the parameter block
58 | ERROR | Expected a valid @param data type, but found type
58 | ERROR | Missing comment for param "$form" at position 1
59 | ERROR | Expected a valid @param data type, but found type
59 | ERROR | Missing comment for param "$form_state" at position 2
63 | ERROR | Expected "foreach (...) {\n"; found "foreach(...) {\n"
65 | ERROR | No space before comment text; expected "// delete here" but
| | found "//delete here"
65 | ERROR | Inline comments must start with a capital letter
65 | ERROR | Inline comments must end in full-stops, exclamation marks, or
| | question marks
70 | ERROR | Whitespace found at end of line
72 | ERROR | Whitespace found at end of line
88 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

The ideea for this module is good.
I tested and it's a problem with the article node type. With other content types works fine.

Fatal error: Function name must be a string in ..\modules\field_ui\field_ui.admin.inc on line 154

As a suggestion: you can put a confirmation page when you delete some fields.

Hi @gaurav.pahua,

please mention the hook type in comment for function "sn_quick_field_delete_form_field_ui_field_overview_form_alter" i.e. hook_form_FORM_ID_alter().

Also mention the in the comment that function "sn_quick_field_delete_multi_delete_submit" is a submit handler for the which form so that whoever view the code, get it easily.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://drupal.org/node/2083109

Project 2: https://drupal.org/node/2070535

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

git clone command added.

Hey Gaurav,

I couldn't find any release blockers however a few points to note.

function sn_quick_field_delete_form_field_ui_field_overview_form_alter, why are you including the form_id argument?

In your form submit function, line 94 you're using field_purge_batch with a static argument. What happens if the number of deleted field records are larger than the static value?

Cheers,

Hi gaurav.pahuja!

I had a look at this project, here is a list of my findings.

- If the module "field_groups" is in use, the field-list-table gets corrupted as you can see on the attached screenshot. Deleting still works perfectly, but as this problem is frequently used, you should consider searching for a solution to this display-issue.

- Way more important: There has to be any kind of confirmation, if submitting a form would delete fields without any way of undoing that action. A checkbox can be checked by mistake and therefore this really is a security-issue. May it be a simple JavaScript-confirmation-diloag "Are you sure you want to delete fields "field_1" and "field_2" in Content Type "CT1"?" or something more complex, if any of these checkboxes are checked, but something has to be added.

- This project does not come up with paragraph 2.3 in the project application checklist (as quoted below), but as this project is really helpful, well done, and can be easily extended in the future, I hope the final reviewer will not be too strict here.

2.3 Ensure your project contains a minimum of handwritten code.
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, that means we can't make sure you're able to write secure code and use Drupal's APIs correctly.
If this is the case, we can't approve you as git vetted user (giving you the permission to create full projects). Either we can see that you have the needed knowledge by other contributions within the drupal community or we can promote it to a full project manually.

- Coding standards seem to be fine, the automated review-tools didn't find anything, neither did I, well done!

As soon as the confirmation-mechanism is added, this module should be ready for the RTBC-status.

regards,
Martin

Okay, you added a really-hard-to-read JavaScript one-liner, but it does what it is supposed to do, so on the security-side this looks a lot better now.

The string "Are you sure you want to delete selected fields?" should be made translatable by using t(), but that's something uncritical and easy to do, so I guess a pro-reviewer should have a look now.

Code looks fine, nice module. Can you add some more detail to your project page?

Thanks for your contribution, gaurav.pahuja!

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.

----
Top Shelf Modules - Crafted, Curated, Contributed.

Removing unused tag.