-
Notifications
You must be signed in to change notification settings - Fork 445
[MDL-80945] Update quizaccess page #1410
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for moodledevdocs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
2b9649f
to
b2f35d3
Compare
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.
Hi,
Firstly thanks for the really detailed update. Much appreciated.
Given this is documentation for a new feature, and we do not land new features in stable versions of Moodle (5.0), there is no need to document it for 5.0. I really appreciate you taking the time to do so, but it won't be landing there.
The only other comment I have is with regards to coding style.
For dev docs we artificially reduce the line length to ensure that code examples fit within the width of the code container. That's around about 105 characters max, but it's often easier to read when it's slightly less than that. I've included a screenshot of what I mean in the generated code:

We also aim to make our coding examples meet current coding style rules because people will use them as copy/paste examples of good practice.
Thanks again.
@@ -34,6 +34,7 @@ | |||
"qbank": "public/question/bank", | |||
"qbehaviour": "public/question/behaviour", | |||
"qformat": "public/question/format", | |||
"quizaccess": "public/mod/quiz/access", |
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.
Please remove this - it is wrong.
The new component is a subplugin of mod_quiz and not a global plugin type.
The content of this file is automatically pulled in.
use context; | ||
|
||
class override_rule extends access_override_rule_base { | ||
|
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.
Please pass this file through the code style checker and fix it.
It's worth noting tha twe also tend to further reduce line length limits to around 80 characters in docs because the way that code scrolls left-to-right makes it hard to view things in context.
* All of the below access_override_rule_base functions to be implemented. | ||
*/ | ||
|
||
public static function add_form_fields(context_module $context, int $overrideid, object $quiz, MoodleQuickForm $mform): void { |
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.
Example of exagerated line length limit:
public static function add_form_fields(context_module $context, int $overrideid, object $quiz, MoodleQuickForm $mform): void { | |
public static function add_form_fields( | |
context_module $context, | |
int $overrideid, | |
object $quiz, | |
MoodleQuickForm $mform, | |
): void { |
public static function validate_form_fields(array $errors, | ||
array $data, array $files, context_module $context): array { |
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.
Note: This coding style is not accepted in Moodle core code any more.
use context_module; | ||
use context; | ||
|
||
class override_rule extends access_override_rule_base { |
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.
Ditto the other exampels
No description provided.