CVS edit link for xMATTx

Hello,

After several unanswered research on the forum, Google and needs on projects, I developed a module to force the download of files or images.

My module implements two formatters with a normal link and an accessible link for use in teaser and full node display or with views module.

I saw that there was a module File Force (http://drupal.org/project/file_force) which has been abandoned but my module is different.

You can see the result (accessible link) here :
- in an content : http://img199.imageshack.us/img199/6696/contentb.png
- with a view : http://img7.imageshack.us/img7/3155/viewk.png

The formatter normal link is the same without accessible part.

If you click on a link the download starts (screen here : http://img686.imageshack.us/img686/8215/downloadap.png).

You can download it here (download_file-6.x-1.0.1.tar.gz): http://www.mediafire.com/xmattx

Thank you

Matthieu

PS: Sorry for my english

Comments

avpaderno’s picture

Issue tags: +Module review

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.

As per http://drupal.org/cvs-application/requirements, the motivation message should be expanded to contain more details about the features of the proposed module/theme; for modules it should include also a comparison with the existing solutions, while for themes a screenshot is also required.

The archive needs to be uploaded in a comment here; without that step, the application cannot continue.

xMATTx’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new3.78 KB

I compared my module to File Force (http://drupal.org/project/file_force) which has been abandoned.

Contrary to the module File Force with which we must call it with the URL download/FILE_PATH, my module allows the user to use with formatting in teaser and full node display or with views module.

The base is very close but the implementation is different. I think it makes too many changes to update the module.

I have attached my module with this comment.

avpaderno’s picture

Thanks for the reply.

xMATTx’s picture

Sorry, my motivation message was not comprehensive enough.

avpaderno’s picture

I should ask why you didn't think to take over the abandoned project, but I understand that taking over an existing project to completely change the code is not easy in most cases, and it can create updating problems to the users, especially when the original maintainer didn't chose the correct datatype for the database fields, in example. I had my experience with that, and I can understand if you didn't try to take the abandoned module over.

xMATTx’s picture

In my case, it does not bother to take over the abandoned project, although I prefer publish my module.

It's just that I will keep the basic function of the module File Force does not remove the features available to the first users.

avpaderno’s picture

Assigned: Unassigned » avpaderno

I will review the code within 4 days.

xMATTx’s picture

I had forgotten one important point my module depends on File Field module while File Force not.

Otherwise, do you know why the Force File module has been abandoned?

avpaderno’s picture

The revisions log just says Abandoned at the request of the maintainer.. I take the maintainer didn't have anymore time to develop new code, or to further develop any code.

avpaderno’s picture

Status: Needs review » Needs work
  1. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the functions, Drupal variables, constants, and global variables defined from the module should be named.
  2. Menu titles, and descriptions should not be passed to t() as that is already done by Drupal core code.
xMATTx’s picture

StatusFileSize
new3.77 KB

For the first point, I corrected the naming of some functions and variables.
Is the theme functions should also be prefixed by the name of the module?
There are other problems as naming because I check my code and I do not see them.

I fixed the second point, I do not know that t() is already done by Drupal core code.

Thank you for your review

avpaderno’s picture

Status: Needs work » Needs review

Theme function names starts with theme_, and then should contain the module name, or there could be conflict with another theme function implemented from a different module; that is the same reason function names need to start with the module name.

Remember to change status, when you upload new code.

xMATTx’s picture

Ok, I'll correct my code and upload it again.

Sorry, I would change the status next time.

avpaderno’s picture

Sorry, I would change the status next time.

There is nothing to be sorry; changing the status gives you more chances I see you updated the code.

xMATTx’s picture

StatusFileSize
new3.78 KB

Normally, I fixed the problems.

xMATTx’s picture

StatusFileSize
new3.92 KB

I added two new formatters a normal link with the type icon (based on File Field) and an accesssible link with the type icon too.

xMATTx’s picture

StatusFileSize
new4.93 KB

I added the translation template (.pot) file and the french translation.

avpaderno’s picture

Status: Needs review » Fixed

Thank you for your contribution! I am going to update your account.
These are some recommended readings to help with excellent maintainership:

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.

Status: Fixed » Closed (fixed)
Issue tags: -Module review

Automatically closed -- issue fixed for 2 weeks with no activity.

avpaderno’s picture

Component: Miscellaneous » new project application
Issue summary: View changes