-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix missing description parameters in external_value constructor all #860
Conversation
✅ Deploy Preview for moodledevdocs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies - they were worse than we all feared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @andrewnicols!
I tried to include your suggestions.
Is it possible you were confusing PARAM_
and VALUE_
prefixes? Please have another look at my changes. From my point of view it now everything should be correct, but of course I can be wrong :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks good to me, and demonstrates both options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I forgot to push my commit, so my comments probably seemed to be pretty confusing :) Now pushed the commit.
No description provided.