Contribution Date
Technology
Contribution Project
Contribution Details
Hi Klausi & stBorchert,
- Use variable_get() with defaults and you can completely remove account_settings_email_attachment.install. : Done
- Code style could be improved : Current code style is coder module and php sniffer version 1.4.6 checked ,please let me know the style which need to follow
- You don't need to use all those if-statements here. Look at the Features module for a clean implementation: Done
- The variable $account_settings_email_attachment_attach_id is empty on first visit, so $account_settings_email_attachment_attach_id[$counter] does not exist: isset check is added
- The variable $counter is not named very well. Please use a name matching the meaning of the variable: Done
- make the upload path for the attachments configurable. Personally I would never store this in the public-files directory.: Changed upload directory to private , directory path configurable I am planning to accomplish it on next iteration
- $account_settings_email_attachment_attach_id[$counter] does not exist: isset check added
- Why do you replace spaces with comma here? This is very uncommon. Additionally you need to replace it back in: Sorry this was due to my old code when I used custom extension validate function. changed.
I am planning to add multiple file upload support and upload directory configurable on next iteration.
Please let me know your thoughts on it.
Thanks
Arijit
Contribution Issue Link
Files count
0
Patches count
0