[RFC|Merged] Modular cron

[igorw]

Has there been any progress on this? Now that request-class has been merged, perhaps that could be used instead of request_var. There are some docs on the wiki. Did you manage to get the unit tests running?

[Oleg]

Regrettably I believe the answer is no. I have not had much time to spend on phpbb.

[igorw]

Thanks for the (non-)update. I have started working on the following, since they are all related to the autoloading:

- Use new naming standards (naderman, bantu)
- prefix classes with phpbb_
- rename tasks to task
- cron task interface has another interface in file (toonarmy)
- Use autoloading (naderman, bantu, evil3, toonarmy)

[igorw]

Another thing is integration with the request class instead of using plain (passed in) superglobals.

I've pushed my changes. They could probably pretty much get squashed into one commit at some point.

http://github.com/igorw/phpbb3/compare/ ... ystem-cron

[Oleg]

Thank you Igor for bringing the diff up to current standards. Here is an updated todo list:

- Tests for new and/or changed code (naderman)
- Add @return and @param documentation to every function (bantu)
- Add documentation to wiki (evil3)
- Use request class instead of plain (passed in) superglobals (igorw)

I would like to see more specific requirements for getting this diff merged. To wit:

- Tests for new and/or changed code: is 100% coverage required? Is one test enough?
- Add @return and @param documentation to every function: this I think is reasonable.
- Add documentation to wiki: is this required? How much documentation is required? I would really rather not use the wiki, would inline documentation be an acceptable replacement?
- Use request class instead of plain (passed in) superglobals: the RFC for this was posted, not to mention implemented, after the work on modular cron was completed. Quite honestly I think modular cron should either be grandfathered or brought up to date by development team members. Thankfully Igor made most of the other changes, for which I am very grateful. If it was not for his work I would not be making this post right now. If the use of request class is declared to be required then I definitely want to see modular cron grandfathered into all yet-to-be-implemented future changes to stop the update treadmill.

I sunk a good number of hours into the modular cron but I am not addicted to it and I will not be spending any more time on it until the total future time investment becomes bounded. If you want it, think about what you absolutely must have and postpone the rest until after the merge.

I apologize for being harsh. I tried to smooth this post down but I found it hard to do. Imagine being ignored for 6 months and then being told to go back to code I have not looked at for half a year to chase changes the dev team implemented while not reviewing my changes and not even saying anything (boilerplate responses do not count). For those not on it, the development team had received a number of messages from me regarding this situation which were not made public.

And don't forget, I am not doing this for myself.

[igorw]

- Tests for new and/or changed code: is 100% coverage required? Is one test enough?

Something in between. cron_manager definitely needs to be tested, to make sure it returns only those tasks that should be run. The individual tasks would also be nice to have tested, if you can find a good way to do it. Maybe Nils can give you more details on what is expected here.

- Add documentation to wiki: is this required? How much documentation is required? I would really rather not use the wiki, would inline documentation be an acceptable replacement?

This is basically for MOD authors, a simple guide telling them how to write a cron task and maybe for administrators how to set up real cron for this. I will take care of this. Consider it assigned to me.

- Use request class instead of plain (passed in) superglobals

While it's highly appreciated if patch submitters do this themselves, the development team being responsible for patches being brought up-to-date sounds reasonable to me. In fact, this change has already been included in my changes from Oct 29. The change was trivial, and in this case grandfathering would not have been an option, because the amount of effort required to make superglobals work would have exceeded the work needed to adjust it. And in the end we want our code to be consistent, especially any code new to 3.1.

I have integrated develop back into my branch (fixed some minor conflicts), so please use that.

Summary: If you could write some tests and add phpdoc comments that would be awesome. I will take care of the wiki documentation.

Thanks.

[igorw]

Looks like Andreas is taking care of the PHPDocs: https://github.com/bantu/phpbb3/compare ... ystem-cron

In which case all we need is tests.

[bantu]

Looks like Andreas is taking care of the PHPDocs: https://github.com/bantu/phpbb3/compare ... ystem-cron

Yup. And I think I'm pretty much done.

[Oleg]

Thank you Igor and Andreas.

I added requested documentation: https://github.com/p/phpbb3/commit/6617 ... e915441813

[igorw]

Looks like the phpdocs were added twice now, since Andreas already had done those changes.

https://github.com/bantu/phpbb3/compare ... ystem-cron
https://github.com/p/phpbb3/compare/php ... ystem-cron

These two changesets should be merged, and I suggest manually applying yours on top of Andreas' changes. I'll take care of it.