Closed ricardoh34 closed 8 hours ago
@ricardoh34 are you able to address my peer review points?
Hello Mark,
I have seen that test fails, but I don´t understand why it happens, and what I have to solve now. Can you give some advice?
Regards, Ricardo
On Tue, Sep 28, 2021 at 3:13 PM Mark Nelson @.***> wrote:
@ricardoh34 https://github.com/ricardoh34 are you able to address my peer review points?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mdjnelson/moodle-mod_customcert/pull/459#issuecomment-929197090, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFOQI2EBG5NXFFAT6QEJJGLUEG5N3ANCNFSM5DLJLERQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
I see now the problem in SQL sentence. I have to change it to fullname, I think
On Tue, Sep 28, 2021 at 3:56 PM Ricardo Herrero @.***> wrote:
Hello Mark,
I have seen that test fails, but I don´t understand why it happens, and what I have to solve now. Can you give some advice?
Regards, Ricardo
On Tue, Sep 28, 2021 at 3:13 PM Mark Nelson @.***> wrote:
@ricardoh34 https://github.com/ricardoh34 are you able to address my peer review points?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mdjnelson/moodle-mod_customcert/pull/459#issuecomment-929197090, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFOQI2EBG5NXFFAT6QEJJGLUEG5N3ANCNFSM5DLJLERQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
I pointed out a few things that need addressing.
I can´t see your peer review points, sorry, where can I see them? I saw the errors in the tests and I have changed the code to address properly fullname column in the database.
I can´t see your peer review points, sorry, where can I see them? I saw the errors in the tests and I have changed the code to address properly fullname column in the database.
Sorry I didn't realise I had to confirm my comments on the code before they would appear.
I will have a look in the next following days but will squash this into one commit.
Closing as there are still issues to address and I have a more efficient solution on solving this.
I decided I would like to give credit to @ricardoh34. Can you please use spaces, instead of tabs and put this in one commit rebased on MOODLE_404_STABLE?
Include name of student and name of the course in the PDF certificate so as to ease organization of the certificates downloaded, solving issue #455