A few quick things that I've noticed that are just small details:
1) The preferred file structure is to have /mod/dimdim/lang rather than copying the lang files over. I know changes were made in 1.9 to allow for this but I need to check if those changes were checked in to 1.8 as well.
2) I noticed that one of the get_string calls in index.php used a double quote and I think it is good to be consistent and just use single quotes throughout. I realize this is just minor almost irrelevant detail but it did catch my eye so I figured I would mention it.
3) I would recommend turning debugging mode on when testing the code so that we catch things like:
Notice: Undefined property: stdClass::$meetingkey in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 115
Notice: Undefined property: stdClass::$hostkey in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 119
Notice: Undefined variable: password in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 136
Notice: Undefined property: stdClass::$privatechat in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 177
Notice: Undefined property: stdClass::$publicchat in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 184
Notice: Undefined property: stdClass::$screencast in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 193
Notice: Undefined property: stdClass::$whiteboard in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 200
Notice: Undefined property: stdClass::$participantlist in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 207
Notice: Undefined property: stdClass::$displaydialinfo in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 270
Notice: Undefined property: stdClass::$interntoll in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 278
Notice: Undefined property: stdClass::$moderatorpasscode in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 282
Notice: Undefined property: stdClass::$attendeepasscode in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 286
These mostly look like just not checking for the existence of a variable before attempting to use it and are easy enough to fix with an if not empty varialbe. I'm not concerned about getting the notices fixed immediately but wanted to mention them while I saw them. The biggest issue is making sure we get the tree structure right from the beginning. If it is OK with you all, I would like to move the lang file underneath the /mod/dimdim folders. Also, I continue to think about the functionality of how the downloads get generated and it may be best to have contrib/plugins/mod/dimdim_40 and contrib/plugins/mod/dimdim_45 as separate trees in CVS but I need to check with Moodle HQs to see if we might be able to have it create separate zip files if we were to use contrib/plugins/mod/dimdim/40 and contrib/plugins/mod/dimdim/45. I'll create a separate issue in the tracker to look at this issue. I appreciate your patience as we look at the best way to get this up and running for you. I just want to make sure that we do it correctly from the beginning.
Peace - Anthony
– personal notes
related to MDL-15892 (get_string issue backport to 1.8?)
auto-create zip files for sub-directories?
Task
Resolved
Major
A few quick things that I've noticed that are just small details:
1) The preferred file structure is to have /mod/dimdim/lang rather than copying the lang files over. I know changes were made in 1.9 to allow for this but I need to check if those changes were checked in to 1.8 as well.
2) I noticed that one of the get_string calls in index.php used a double quote and I think it is good to be consistent and just use single quotes throughout. I realize this is just minor almost irrelevant detail but it did catch my eye so I figured I would mention it.
3) I would recommend turning debugging mode on when testing the code so that we catch things like:
Notice: Undefined property: stdClass::$meetingkey in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 115
Notice: Undefined property: stdClass::$hostkey in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 119
Notice: Undefined variable: password in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 136
Notice: Undefined property: stdClass::$privatechat in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 177
Notice: Undefined property: stdClass::$publicchat in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 184
Notice: Undefined property: stdClass::$screencast in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 193
Notice: Undefined property: stdClass::$whiteboard in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 200
Notice: Undefined property: stdClass::$participantlist in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 207
Notice: Undefined property: stdClass::$displaydialinfo in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 270
Notice: Undefined property: stdClass::$interntoll in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 278
Notice: Undefined property: stdClass::$moderatorpasscode in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 282
Notice: Undefined property: stdClass::$attendeepasscode in /home/arborrow/Moodle/code/19stable/mod/dimdim/mod.html on line 286
These mostly look like just not checking for the existence of a variable before attempting to use it and are easy enough to fix with an if not empty varialbe. I'm not concerned about getting the notices fixed immediately but wanted to mention them while I saw them. The biggest issue is making sure we get the tree structure right from the beginning. If it is OK with you all, I would like to move the lang file underneath the /mod/dimdim folders. Also, I continue to think about the functionality of how the downloads get generated and it may be best to have contrib/plugins/mod/dimdim_40 and contrib/plugins/mod/dimdim_45 as separate trees in CVS but I need to check with Moodle HQs to see if we might be able to have it create separate zip files if we were to use contrib/plugins/mod/dimdim/40 and contrib/plugins/mod/dimdim/45. I'll create a separate issue in the tracker to look at this issue. I appreciate your patience as we look at the best way to get this up and running for you. I just want to make sure that we do it correctly from the beginning.
Peace - Anthony
– personal notes
related to
MDL-15892(get_string issue backport to 1.8?)auto-create zip files for sub-directories?
MDL-15892(get_string issue backport to 1.8?) auto-create zip files for sub-directories?