-
Notifications
You must be signed in to change notification settings - Fork 3
Update format_weeksrev for Moodle 4.5 #6
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: master
Are you sure you want to change the base?
Conversation
Resolve "Posodobitev format_weeksrev za verzijo 4.5" Closes ak4t0sh#1 See merge request ucilnice/format_weeksrev!1
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,
Thanks for sharing this fix with us @vanjadi and for your amazing work on this @1katoda !
Tested it works as expected 👍
I have reported some minor changes to do before merging this PR.
Once done I'll release a new version.
Also if you have a moodle.org account I can add you as contributor for this plugin (I'll need your moodle username) if you want to.
I also noted in your git history that you have a slovenian translation :) Ideally could you add it to AMOS ? (https://lang.moodle.org/)
lib.php
Outdated
@@ -189,9 +176,9 @@ public function extend_course_navigation($navigation, navigation_node $node) { | |||
* | |||
* @return array This will be passed in ajax respose | |||
*/ | |||
public function ajax_section_move() { | |||
function ajax_section_move() { |
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.
why removing method visibility ?
lib.php
Outdated
@@ -158,7 +145,7 @@ public function supports_ajax() { | |||
*/ | |||
public function extend_course_navigation($navigation, navigation_node $node) { | |||
global $PAGE; | |||
// If section is specified in course/view.php, make sure it is expanded in navigation. | |||
// if section is specified in course/view.php, make sure it is expanded in navigation |
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.
To revert. Comments must begin by a capital letter and must end with a punctuation character.
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.
Why are you replacing some short array syntax to long array syntax ?
Short array syntax [] is recommended by the guidelines https://moodledev.io/general/development/policies/codingstyle#array-syntax
Could you revert those changes please ?
version.php
Outdated
* based on code by Mat Cannings | ||
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later | ||
*/ | ||
|
||
defined('MOODLE_INTERNAL') || die(); | ||
|
||
$plugin->version = 2019020300; // The current plugin version (Date: YYYYMMDDXX). | ||
$plugin->requires = 2017111300; // Requires this Moodle version. | ||
$plugin->version = 2025032600; // The current plugin version (Date: YYYYMMDDXX). |
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.
Could you revert those changes please ? I'll take care to update it myself during release process.
version.php
Outdated
$plugin->component = 'format_weeksrev'; // Full name of the plugin (used for diagnostics). | ||
$plugin->release = '3.2.0'; | ||
$plugin->release = '3.2.1'; |
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.
Could you revert those changes please ? I'll take care to update it myself during release process.
BTW as in the "requires" we are dropping support for moodle version prior to 4.5 the release number will be 4.0.0 ;)
CHANGES.md
Outdated
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.
Could you revert those changes please ? I'll take care to update it myself during release process.
README.md
Outdated
It is a fork, compatible with **Moodle 4.0+**. | ||
|
||
Forked from: https://github.com/ak4t0sh/moodle-format_weeksrev | ||
|
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.
Could you revert this changes please ?
version.php
Outdated
@@ -19,15 +19,16 @@ | |||
* | |||
* @package format | |||
* @subpackage weeksrev | |||
* @copyright 2018 Arnaud Trouvé <[email protected]> | |||
* @copyright 2024 1katoda |
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.
to revert please
Hi, thanks for the review! Credits for most of the changes go to @vanjadi, she contributed the most code to this PR (and implemented the requested changes), my Moodle account name is the same as my Github account. |
Hi, Requested changes have been implemented and slovenian translation has been added to AMOS. My Moodle account username is also the same as my Github account (vanjadi). |
This update ensures compatibility with Moodle 4.5 by adjusting the plugin's structure and updating the renderer. Specifically, the renderer has been moved under classes/output, and necessary changes have been made to routing and content rendering. Additionally, deprecated components have been replaced with core_course output components, as required since Moodle 4.0. These changes align the plugin with Moodle’s latest architecture while maintaining its existing functionality.