search

Panopto / Moodle-2.0-plugin-for-Panopto

Panopto's integration with the Moodle LMS.
http://www.panopto.com
GNU General Public License v3.0
18 stars 40 forks source link

General update including code guidelines and images #3

Closed andrewnicols closed 11 years ago

andrewnicols commented 12 years ago

I've updated the code to comply better with the Moodle coding guidelines and make it (IMHO) easier to read.

I've also moved the feed_icon image to the pix folder to allow it to be themed more easily.

Panopto commented 12 years ago

Hi Andrew,

Thanks for the updates. Sorry for the slow response, we're in a mini crunch right now, so it will probably be a week before I get a chance for a proper review and merge.

I expect we'll take the themeability change. Regarding the coding guidelines change, can you point me to the coding guidelines doc? Our internal coding style has been to keep braces on new lines, so that was our default here. Are there other changes besides brace style here? Also, note that there are code files in /lib which should also be considered.

Thanks! -Greg

andrewnicols commented 12 years ago

Hi Greg,

Thanks for getting back to me and no worries about the speed of your response - I'm in the middle of a similar bug squashing sprint.

The moodle coding guidelines can be found at http://docs.moodle.org/dev/Coding_style. From memory, the only other change I made was to convert all files to have UNIX line encodings.

If this change is accepted, I'll probably spend some time (when I have a moment) converting the block to use moodle renderers so that theme-specific changes can be made to the rendering. I suspect that I'll also make some other changes to add additional functionality and enhancements. One of the ones I was hoping to do was to create a new capability so that we can limit which users have access to panopto as instructors ( https://github.com/andrewnicols/Moodle-2.0-Plugin-for-Panopto/blob/master/lib/panopto_data.php#L114 ).

Thanks for pointing out /lib directory - if you're happy to accept the coding guideline changes, I'll submit relevant changes for the lib files too.

Thanks again,

Andrew

On 2 April 2012 19:50, Panopto, Inc. < reply@reply.github.com

wrote:

Hi Andrew,

Thanks for the updates. Sorry for the slow response, we're in a mini crunch right now, so it will probably be a week before I get a chance for a proper review and merge.

I expect we'll take the themeability change. Regarding the coding guidelines change, can you point me to the coding guidelines doc? Our internal coding style has been to keep braces on new lines, so that was our default here. Are there other changes besides brace style here? Also, note that there are code files in /lib which should also be considered.

Thanks! -Greg

Reply to this email directly or view it on GitHub:

https://github.com/Panopto/Moodle-2.0-Plugin-for-Panopto/pull/3#issuecomment-4885509

Systems Developer

e: andrew.nicols@luns.net.uk im: a.nicols@jabber.lancs.ac.uk t: +44 (0)1524 5 10147

Lancaster University Network Services is a limited company registered in England and Wales. Registered number: 04311892. Registered office: University House, Lancaster University, Lancaster, LA1 4YW

Panopto commented 11 years ago

Thanks Andrew! I am putting together a very similar style update change for the Moodle 2.4 update, and this helped me get started. Thanks!