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

Removed legacy cron function #576

Merged

Conversation

sammarshallou
Copy link
Contributor

In Moodle 4.2, cron_trace_time_and_memory has been replaced by a different function in a namespaced location, and you get deprecation warnings for including cronlib.php as well as for calling the function.

This change removes the old cron function in lib.php, and changes the code that calls cron_trace_time_and_memory so that it uses the old method prior to 4.2, and the new one after.

Note: This is a duplicate of #568 but I wrote the code before I saw that one! While it is safe in the MOODLE_310_STABLE branch, to remove the cron function from lib.php (as 3.10 already supported task API), it is not safe to change the cron_trace_time_and_memory function because the new \core\cron class does not exist in 3.10, or any version until 4.2. So I added a version check.

@brendanheywood
Copy link
Contributor

thanks @sammarshallou - I think this is good to land without a new stable. Not sure about the red ci that seems unrelated

@brendanheywood
Copy link
Contributor

@sammarshallou does the 'Could not use "Hint_ResultPrinter" as printer: class does not exist' error mean anything to you?

@sammarshallou
Copy link
Contributor Author

@brendanheywood I saw it but sorry, no, I don't know how the CI infrastructure here is supposed to work and I don't understand it at all! I presume these errors are unrelated to the change here...

I did a quick search and found this issue in which remoing that class (or one with the same name...) was discussed because it apparently wasn't needed, but that was done way back in Moodle 3.10 so probably not related https://tracker.moodle.org/browse/MDL-67673

It looks like, at least in my repository, the same error occurs if I just push the MOODLE_310_STABLE branch without my change.

https://github.com/sammarshallou/moodle-tool_objectfs/actions/runs/5961359997

@aspark21
Copy link

the change to the phpunit.xml was applied last week so with a rebase those errors should clear up.

@sammarshallou sammarshallou force-pushed the remove-cronlib-inclusion branch from 33f92e8 to bcafde6 Compare September 13, 2023 16:09
@sammarshallou
Copy link
Contributor Author

@aspark21 Thanks! I've rebased, let's see.

This change removes the old cron function in lib.php which is no longer needed
in Moodle 4.2.
@sammarshallou sammarshallou force-pushed the remove-cronlib-inclusion branch from bcafde6 to dd0d2a4 Compare July 29, 2024 10:57
@sammarshallou sammarshallou changed the base branch from MOODLE_310_STABLE to MOODLE_402_STABLE July 29, 2024 10:57
@sammarshallou sammarshallou changed the title Removed legacy cron function and deprecated cron_trace_time_and_memory Removed legacy cron function Jul 29, 2024
@sammarshallou
Copy link
Contributor Author

Somebody already just deleted the trace_time_and_memory call in the MOODLE_402_STABLE branch, so I've changed this PR to remove that part and just delete the unused cron function and corresponding test. (I'm not sure if the existence of this function actually causes problems but it's a bit pointless.)

@danmarsden danmarsden merged commit 0a55f98 into catalyst:MOODLE_402_STABLE Jul 29, 2024
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.

4 participants