Skip to content
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

68 Fix Fatal error declaration of assert_same_xml #69

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

leonstr
Copy link

@leonstr leonstr commented Jul 1, 2024

MDL-81581 moved assert_same_xml() into \question_testcase so any implementation a subclass must match this definition.

While removing type declarations for assert_same_xml()'s parameters would fix this, this PR additionally uses the parent's implementation if present, falling back to the existing implementation if if it isn't to avoid making the plugin dependent on Moodle versions >= 4.4.1. This is a little clunky but makes explicit that the core code change means the existing implementation is considered deprecated.

(In core, MDL-81581 removed qtype plugins' implementations of assert_same_xml(), but we can't do that without making qtype_pmatch dependent on Moodle >= 4.4.1).

MDL-81581 moved assert_same_xml() into \question_testcase so any
implementation a subclass must match this definition.
@leonstr leonstr mentioned this pull request Jul 2, 2024
Copy link

@daniil-berg daniil-berg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me and fairly straightforward. Hope this gets merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants