joomla-cms
joomla-cms copied to clipboard
joomla
[5.2] [4.1 ScheduledTasks]Edits for run task
Pull Request for Issue #36453 .
Summary of Changes
Edits for run task.
Please review. Thanks
Please check whether you have taken over all comments from your previous PRs. Furthermore, I have the impression, but still have to test it, that again not all occurrences have been renamed. Please also check this with a full text search over the whole repo. And please keep the code up to date.
But sir , it is working fine after running the command (npm run build:js )
Should this also be adapted?
https://github.com/joomla/joomla-cms/blob/838845941106a2b93eed1d304ad97f3d152cb3a9/plugins/system/schedulerunner/schedulerunner.php#L92
and
https://github.com/joomla/joomla-cms/blob/838845941106a2b93eed1d304ad97f3d152cb3a9/plugins/system/schedulerunner/schedulerunner.php#L255
Perhaps with runTaskCron
And maybe also replace onAjaxRunSchedulerTest with onAjaxRunSchedulerTask?
@ditsuke what do you think about this improvements?
Please check this two code comments.
https://github.com/joomla/joomla-cms/blob/42f7f89788384ca08ec3a43d46a7e736a2777a48/plugins/system/schedulerunner/schedulerunner.php#L243
https://github.com/joomla/joomla-cms/blob/42f7f89788384ca08ec3a43d46a7e736a2777a48/plugins/system/schedulerunner/schedulerunner.php#L297
Also, please check the occurrences of 'test-task'. This concerns file names, function calls and declarations.
@niharikamahajan02 sorry for all the comments but it's best to clean it straight away.
Also, please check the occurrences of 'test-task'. This concerns file names, function calls and declarations.
@niharikamahajan02 sorry for all the comments but it's best to clean it straight away.
No problem sir 😄 and yes it's better to clean them straight away now
Yes ,I would rename the file names, function calls and declarations also 👍
@tecpromotion
@ditsuke what do you think about this improvements?
Thanks for tagging! I really like the changes so far.
@niharikamahajan02
Yes sir , changed the file name now.
Nicely done! You can resolve the suggestions I left above from your end.
Actually for resolving the conflict I am doing git pull, and it's saying already up to date. Even after that new changes made in administrator/components/com_scheduler/tmpl/tasks/default.php( which is causing conflict ) are not getting reflected.
Actually for resolving the conflict I am doing git pull, and it's saying already up to date. Even after that new changes made in administrator/components/com_scheduler/tmpl/tasks/default.php( which is causing conflict ) are not getting reflected.
I've opened a PR to update your branch to 4.1-dev and resolve conflicts.
PTAL https://github.com/niharikamahajan02/joomla-cms/pull/1.
Ok , so do I need to do something for now?
Please merge the PR. It's on your fork.
This pull requests has been automatically converted to the PSR-12 coding standard.
I rebased the PR and added back removed language strings and deprecated them.
@niharikamahajan02 please solve the unresolved conversations of this PR. I would like to see this merged into a following Joomla version, while Run Test is the correct name for the action the button is doing
please review it once , I think there are some changes to be done
please review it once , I think there are some changes to be done
@niharikamahajan02 It needs to resolve merge conflicts in files build/media_source/com_scheduler/joomla.asset.json and plugins/system/schedulerunner/schedulerunner.php. Maybe just use the clean files from the 5.1-dev branch and apply your changes again on them.
but they are already having "run test" in 5.1-dev
@niharikamahajan02 What do you mean? Drone tests are failing, and if you go to the bottom of this PR on GitHub you clearly see there are the 2 mentioned files which have merge conflicts. If you don't know how to solve them we can help, but maybe you can try it yourself first like I have described.