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

Prevent collapsable sections conflicting (main branch) #228

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

Conversation

james-cnz
Copy link

I'd like to apply a fix to Moodle:
Collapsable sections can conflict, and trigger each other https://tracker.moodle.org/browse/MDL-82679
But it would break current versions of Grid format.
This patch is to prevent that.
The extra file would only be temporary, and could be removed again in the next version.

@gjb2048
Copy link
Collaborator

gjb2048 commented Nov 12, 2024

@james-cnz Thanks for the heads up about MDL-82679 however:

  • It takes me 20/30 mins to do a release of the format on Moodle dot org, increasing with the number of branches. This I would have to do twice.
  • Moodle tracker issues can take weeks / months and even years / decades before they are accepted and integrated into core and it hasn't even been peer reviewed yet! Let alone integration reviewed where changes / nature of the fix may change, thus invalidating the fix you propose to be applied to the Grid format.
  • When MDL-82679 does land then all I need to do is patch one line in one file and set the minimum version of Moodle in the version.php file and then release as per what is accepted in the tracker issue at the time.

Therefore, I'll wait until MDL-82679 is integrated and released before I expend any time.

@james-cnz
Copy link
Author

Hi @gjb2048,
MDL-82679 has progressed, and is now waiting for component lead review. Could this issue be reopened?

@gjb2048
Copy link
Collaborator

gjb2048 commented Jan 22, 2025

Hi @gjb2048,
MDL-82679 has progressed, and is now waiting for component lead review. Could this issue be reopened?

Hi @james-cnz,

No. I am not spending unnecessary time on this. When MDL-82679 has been 'Integrated' into the stable branches then I will consider it's implications.

If you need it to prove a point / solve your problem then make your own fork.

G

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

Successfully merging this pull request may close these issues.

2 participants