Penny Leach: 12:42:42What does moodle hq think about me ignoring the 1.9 thing and doing http://tracker.moodle.org/browse/MDL-6605 instead ?Penny Leach: 12:42:47feels rebelliousPenny Leach: 12:45:04crap, I need to merge my gsoc student code tooEloy Lafuente: 12:46:29Really radical! Absolutely!Penny Leach: 12:46:53really ?Penny Leach: 12:46:56you reckon ?Penny Leach: 12:47:08Eloy - I'd like to discuss it with you a little bit,Eloy Lafuente: 12:47:09reckon?Penny Leach: 12:47:20last time I did it it broke backwards compatibility with a few functions because tehy need a new argumentEloy Lafuente: 12:47:22yep, me too.Martín Langhoff: 12:47:43at moot uk, talking with tim we came up[ with a backwards compat strategy for that...Martín Langhoff: 12:47:56with a few other added benefits...Eloy Lafuente: 12:47:59really? I'm all earsMartín Langhoff: 12:48:17keep current dmllib functions as tehy are for bw compatMartín Langhoff: 12:49:07and have a new Moodle-style global DB object with identical methods - subclassing or wrapping around AdoDBEloy Lafuente: 12:49:55well, that's the design view. nothing against that (nor the opposite).Eloy Lafuente: 12:49:59;-)Eloy Lafuente: 12:50:14what about compatibility?Penny Leach: 12:50:28martin - I don't understand your proposalEloy Lafuente: 12:50:48the key here is that dmllib themselves aren't compatible nor incompatible.Martín Langhoff: 12:50:54this means that:
- insert_record() can coexist wuth $mdb->insert_record()
- during transition, scripts can declare - before they include config.php - that they do not want addslashes/magicquotesPenny Leach: 12:50:57do you mean go through and change all current calls to (eg get_record) with $newdb->get_record with the different parameters ?Eloy Lafuente: 12:50:59IMO the incompatibility is in:Eloy Lafuente: 12:51:05addslahesEloy Lafuente: 12:51:13and it's spreaded in ALL codePenny Leach: 12:51:21Eloy - you have to switch it to detect it ON and turn it OFF, and remove in codeMartín Langhoff: 12:51:24we still have to go through and study all through the code things like addslashes - as eloy mentionsPenny Leach: 12:51:31but that doesn't break backwards compat, the problem is the function parametersPenny Leach: 12:51:39you need to start passing $values to SOME of themPenny Leach: 12:51:40like ..Penny Leach: 12:51:48finds patchEloy Lafuente: 12:51:56yep, yep. Penny. Apart from the array of placeholders...Penny Leach: 12:52:04http://git.catalyst.net.nz/gitweb?p=elgg.git;a=commitdiff;h=553765dbdba08162745fe10710ac20153f3c12d2Penny Leach: 12:52:10^^^ very old patch, but gives you an ideaPenny Leach: 12:52:26-function count_records_select($table, $select='', $countitem='COUNT(*)') {
+function count_records_select($table, $select='', $values=null, $countitem='COUNT(*)') {Penny Leach: 12:52:30(eg)Eloy Lafuente: 12:52:31the first thing to decide is HOW are we going to keep compatibility (about slashes) or no.Penny Leach: 12:52:57you have to switch the workaround in lib/setup.php to detect it on and go through and stripslashes everythingEloy Lafuente: 12:52:58yep, yep. I think that all we...Penny Leach: 12:53:02and grep code for addslashesEloy Lafuente: 12:53:12think the same about placeholders. It's the easy part IMO.Eloy Lafuente: 12:53:19it's addslashes. yep.Martín Langhoff: 12:53:21... would this not help the transition and support old code as well?Penny Leach: 12:53:24Eloy - even if it changes the argument order ?Martín Langhoff: 12:53:41it measn that we are free to change function prototypes more freelyEloy Lafuente: 12:53:49yep, yep-Penny Leach: 12:54:00oh ok I thought that would be the issue :)Eloy Lafuente: 12:54:06agree. I love OOP so I haven't nothing against that.Eloy Lafuente: 12:54:23What I don't as a correct solution...Penny Leach: 12:54:28I don't see how changing the order of arguments in a function definition relates to OOP ?Martín Langhoff: 12:54:37and once we have that - we can also use MoodleDB for other external DBs, like in Auth/DB, etc -- (if we want to)Eloy Lafuente: 12:54:41is to PLAGUE code with conditional addslashes !Penny Leach: 12:54:51Eloy - it must GO totallyPenny Leach: 12:54:54NO MORE ADDSLASHESPenny Leach: 12:54:57:)Eloy Lafuente: 12:55:02That makes me think if wouldn't be better to BREAK backwards completely.Martín Langhoff: 12:55:04Penny - having the 2 apis in parallel means we can break more with the old APIMartín Langhoff: 12:55:16as we'll have to walk the code anywayPenny Leach: 12:55:28I think having 2 APIs is a bad ideaEloy Lafuente: 12:55:34yep, agree.Penny Leach: 12:55:41Eloy - with who? ;)Eloy Lafuente: 12:55:462.0 release shouldn't have both.Martín Langhoff: 12:55:55it's just a transition thing - it will be hard, but it will give us bw compat.Eloy Lafuente: 12:55:56only one IMO.Eloy Lafuente: 12:56:07sorry Martin....Martín Langhoff: 12:56:22and it is a good chance to rearrange the method params.Eloy Lafuente: 12:56:41what I don't understand.... is how are we going to kill all those addslashes currently in code (or change them to conditionals).Eloy Lafuente: 12:56:53IMO that's a horrible hack.Eloy Lafuente: 12:57:20or are you talking... only... about contrib modules being able to use the old API ?Eloy Lafuente: 12:57:30and ALL the core using the new ONE?Eloy Lafuente: 12:57:46or what?Martín Langhoff: 12:57:52of course, I'd want to ship v2 only using the new API ;-)Martín Langhoff: 12:58:12but have the old calls available - perhaps force an option to enable itEloy Lafuente: 12:58:22(puke)Martín Langhoff: 12:58:33it also means that while we develop the code for teh transition, we have something working all the timeEloy Lafuente: 12:58:442.0 is about to break things IMO. And that's a good a justified break IMO.Penny Leach: 12:58:51I agreeMartín Langhoff: 12:58:53avoid having to prepare one huge patch against all of moodlePenny Leach: 12:58:542.0 is the time to do itPenny Leach: 12:59:02Martin -Penny Leach: 12:59:14I think it's worth doing properly if we're going to do itPenny Leach: 12:59:21keeping 2 APIs encourages lazinessPenny Leach: 12:59:48and it introduces the possibility of security nightmareEloy Lafuente: 12:59:54one 1-month branch should be able to do all the BIG job IMO.Eloy Lafuente: 13:00:02then merge and polish.Penny Leach: 13:00:06if you're not either always knowing you have addslashes or knowing you have placeholders... there's SO many entry points for security bugsEloy Lafuente: 13:00:42after all... the BIG job includes:Martín Langhoff: 13:00:49PEnny - no, security problems with the 2 APIs - the legacy API simply does a stripslashes() and passes on to the new APIPenny Leach: 13:01:05erPenny Leach: 13:01:16if each script has to say whether it wants its variables addslashed or not, that's not trueMartín Langhoff: 13:01:25the challenge is to do this without breaking old modules completelyPenny Leach: 13:01:33forgetting to say ZOMG_BACKWARDSCOMAPTIBLITY_ADDSLASHES_WORKAROUND_PLZ is easyEloy Lafuente: 13:01:331) Create the new API and test it 300%.
2) Repetitive addslahes() killing.
3) Repetitiveencapsulate placeholder calls.Eloy Lafuente: 13:02:022 & 3 can be performed in branch relatively quickly.Eloy Lafuente: 13:02:091 can be done in paralell.Eloy Lafuente: 13:02:16in TRUNKPenny Leach: 13:02:19Eloy - y epEloy Lafuente: 13:02:54or... in other words... what's the advantage of keeping both APIs?Eloy Lafuente: 13:03:06I cannot see that.Martín Langhoff: 13:03:23the advantage is called contrib, private modules, etcPenny Leach: 13:03:39you have to break backwards compatibility sometimesPenny Leach: 13:03:46if the benefit is worth itPenny Leach: 13:03:50moodle 1.6 broke themes rememberPenny Leach: 13:03:52so much better nowEloy Lafuente: 13:03:59and UTF8Penny Leach: 13:04:04oh yeah totallyEloy Lafuente: 13:04:10and install/upgradePenny Leach: 13:04:18well there was a transition for thatPenny Leach: 13:04:25postgres7.* and mysql.* stuck around for a releaseEloy Lafuente: 13:04:44yep. The key here, IMO is about to perform 1) (publish the new API) ASAP.Penny Leach: 13:04:46moving to use referential integrity will almost CERTAINLY break backwards compatibilityMartín Langhoff: 13:04:46install/upgrade got 2 releasesPenny Leach: 13:05:01eg code deleting things from 2 tables in incorrect orderEloy Lafuente: 13:05:02That will provide people with ... 9 months... to migrate?Penny Leach: 13:05:07won't work with FKsEloy Lafuente: 13:05:13IMO far enough for maintained modules.Penny Leach: 13:05:16Eloy - I agree, which is why I propose doing it now, early, get it inEloy Lafuente: 13:06:11that's only my opinion, of course. But I cannot see really both APIs working together in a correct way.Penny Leach: 13:06:27lets wait for MD to come online for his input too