Learning through bug fixing

Learning through bug fixing

door Jun Yamog -
Aantal antwoorden: 5
Hi,

I am new to moodle, I am trying to learn moodle.  I figured the best way for me to learn is to help on some bugs.

I picked this bug for me to learn some stuff about moodle

http://moodle.org/bugs/bug.php?op=show&bugid=4122

I have created a patch for this, Martin Langhoff told me the patch is fine.  I have tested it to run on both pgsql and mysql, clean install and running the upgrade scripts.

Do you guys think the patch is ok?

Jun
Gemiddelde van de beoordelingen:  -
Als antwoord op Jun Yamog

Re: Learning through bug fixing

door Tim Hunt -
Foto van Core developers Foto van Documentation writers Foto van Particularly helpful Moodlers Foto van Peer reviewers Foto van Plugin developers
Minor nit:

In Lines 10 and 11 of your patch, you are using tabs instead of spaces to indent the lines. Moodle coding standard says use spaces.

Ditto, lines 41-45.

It's not important, but do you have any particular reason for setting $version to 2006022001 rather than 2006022000 in version.php?
Gemiddelde van de beoordelingen:  -
Als antwoord op Tim Hunt

Re: Learning through bug fixing

door Jun Yamog -
Hi Tim,

Sorry about the tabs, I must have set emacs to use spaces after making the patch.  I set it 2006022001 since I revised my upgrade script, anyway its internal to me.  I have now set the version to 2006022000 in version.php and the respective upgrade scripts.

Reattached another patch file, I will commit maybe tomorrow if the patch is ok.  As well as add a comment on the bugs.moodle.org

Jun
Gemiddelde van de beoordelingen:  -
Als antwoord op Jun Yamog

Re: Learning through bug fixing

door Tim Hunt -
Foto van Core developers Foto van Documentation writers Foto van Particularly helpful Moodlers Foto van Peer reviewers Foto van Plugin developers
The patch looks fine to me now. Thanks for taking such care over the code you contribute.
Gemiddelde van de beoordelingen:  -
Als antwoord op Tim Hunt

Re: Learning through bug fixing

door Jun Yamog -
Thanks Tim.  I have updated the bug report to include a link here and attached the patch.  I will just commit next week at catalyst to allow more time.
Gemiddelde van de beoordelingen:  -