All functions required by Meebo are implemented. Meebo optional functions are either implemented or stubbed, depending on whether or not my project currently needed the functionality. The stubbed, optional functions are clearly marked as NOT YET IMPLEMENTED. (Yes, I did read the note about submitting incomplete code. The modules are complete and functional, implementing all of the required Meebo functions. The stubs are placeholders for anybody who later wants to implement the optional functions.)
I did find reference to another contributed Meebo module (http://drupal.org/project/meebo). I examined the code, and found no duplication with my work. I contacted the contributor of that module, discussed my work, and there is currently no overlap in implementation. He will be able to use my work, saving some effort on his side.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | meebo_modules.tar_.gz | 6.55 KB | Anonymous (not verified) |
| #1 | meebo_modules.tar_.gz | 8.24 KB | Anonymous (not verified) |
Comments
Comment #1
Anonymous (not verified) commentedComment #2
avpadernoComment #3
kharbat commentedHi bksmithey.. I have installed your modules and found the following issues..
1- Your info files are missing core version
2- I got this warning on module enable: warning: Missing argument 1 for meebo_partner_api_menu() in sites/all/modules/meebo_partner_api/meebo_partner_api.module on line 61.
3- The following pages were not found: admin/meebo/meebo_api AND admin/meebo/meebo_partner_api
4- I wonder what is the may_cache parameter in hook_menu for..
Good job though.. looking forward to use your final release..
Comment #4
avpaderno@kharbat: The module is for Drupal 5; that is why
hook_menu()uses a parameter, and the .info file doesn't contain any information about the version of Drupal required. If you installed it on Drupal 6, that would explain the errors you have gotten.I am changing the status back.
Comment #5
Anonymous (not verified) commentedYes, sorry kharbat, I should have mentioned that the module is currently coded for drupal 5 (our site is still on version 5).
kiamlaluno's comment explains your issues (2) and (4) (the hook_menu argument), and issue (1) (the core version)
regarding the admin pages, I'm not sure what the problem is, but looking at the array keys for the items returned by the hook_menu, they have changed a bit between 5->6. For example, my code has 'callback' and 'callback arguments' while it looks like these are called 'page callback' and 'page arguments' in drupal 6. I'm not sure if that is the root of the problem, but you might try making a quick pass through the hook_menu functions in both modules and updating as necessary.
I think porting the modules to drupal 6 should be pretty easy, as the modules are not very big. But I don't have a system here running 6, so I haven't had a chance to do a port and test it.
Good luck! After you successfully port from 5->6, let me know if you have any questions/issues configuring the modules and testing/integrating with Meebo. I found the curl command line very useful for testing both modules before I went live with Meebo.
Comment #6
avpadernoI am adding an additional review tag, so other users know for which Drupal version this module is for.
Comment #7
avpadernoRemove all the debug code.
Comment #8
Anonymous (not verified) commentedTest/debug code has been removed, new tarball is attached
Comment #9
Anonymous (not verified) commentedComment #10
zzolo commentedHi @bksmithey, I am curious about how this module compares to the existing meebo module. Your description above says there is no overlap, but then you say that the existing module could use your code. So, if you could go into details about the specific functions and features that are different between the two modules. Thanks.
--
Note: Please be patient with the CVS application process. It is all done by volunteers. Our goal is not to be arbitrarily slow or meticulous. Our goal is to get you CVS access and ensure that you are and will become a more responsible Drupal contributor.
Comment #11
Anonymous (not verified) commentedHi @zzolo -- it's been a while, I don't really remember any details about the other module. I believe it was at a higher level, and would sit on top of my code. Seriously, it's been so long that I don't remember. You might try checking with user kharbat (comment #3 above), he is the author of the other module. I did contact him early on, described my modules to him, asked if there was any conflict, and he said no, that he'd actually be able to use my modules in his work and it would save him some effort. He may be able to describe the differences between the modules.
I'm actually no longer working for the company where I was doing drupal work, so I don't really have any remaining interest in this code. If it's useful to anybody, feel free to take it and run with it. The latest tarball (attached at comment #8) is fully functional.
Comment #12
zzolo commented@bksmithey, well, it sounds like you are stopping this application process, so I am marking as do not fix. Please feel free to re-apply with a different module that you have more invested interest in.
Just to clarify, the idea is to put the burden of proof on the applicant (you) that they have done due diligence in ensuring that their module does not duplicate, and where applicable, combines efforts with other modules. Ideally, you could link to an issue where this discussion already happened.
Best of luck, and we look forward to a new application!
--
Note: Please be patient with the CVS application process. It is all done by volunteers. Our goal is not to be arbitrarily slow or meticulous. Our goal is to get you CVS access and ensure that you are and will become a more responsible Drupal contributor. For a quick reference on what I look for in a code review, please see this article.