[RFC|Merged] Modular cron

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Wed Oct 13, 2010 10:02 pm

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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Modular cron

Post by Oleg » Thu Oct 14, 2010 3:03 am

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

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Thu Oct 28, 2010 9:02 pm

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)

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Fri Oct 29, 2010 11:24 am

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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Modular cron

Post by Oleg » Tue Dec 14, 2010 5:13 am

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.

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Wed Dec 15, 2010 10:02 am

nn- wrote:- 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.
nn- wrote:- 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.
nn- wrote:- 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.

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Thu Dec 16, 2010 6:16 am

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.

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Modular cron

Post by bantu » Fri Dec 17, 2010 12:55 am

igorw wrote: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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Modular cron

Post by Oleg » Fri Dec 17, 2010 2:11 pm

Thank you Igor and Andreas.

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

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Fri Dec 17, 2010 2:40 pm

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.

Post Reply