IRCLogs20110428 (diff)

You must first sign up to be able to contribute.

Changes from Version 1 of IRCLogs20110428

lsmith (IP:
04/29/11 00:31:02 (7 years ago)



  • IRCLogs20110428

    v0 v1  
     1= Summary = 
     5== Added `compile_classes` option to skip compiling classes == 
     7== postfixing all compiled PHP files with ".cache" == 
     9== removed exit calls == 
     11== Renaming loadUserByUsername() == 
     13== Form layer feedback == 
     15== Annotation aliases, namespaces, and conflicts in general == 
     17== Cache warming == 
     20[16:59] <lsmith> so everybody ready to meet today? 
     21[16:59] henrikbjorn quit IRC (Read error: Connection reset by peer) 
     22[16:59] henrikbjorn joined the channel 
     23[16:59] <lsmith> jmikola|w, kriswallsmith, johannes_, henrikbjorn, fabpot, Seldaek, johnwards 
     24[16:59] <johnwards> oh its thursday already 
     25[16:59] rande joined the channel 
     26[16:59] <kriswallsmith> hi 
     27[17:00] Phoop joined the channel 
     28[17:00] <kriswallsmith> opensky is in a meeting, probably another 5 minutes 
     29[17:00] <fabpot> hi all 
     30[17:00] cordoval_ joined the channel 
     31[17:00] codecowboy joined the channel 
     32[17:00] <lsmith> guess the #1 topic is fixed already: Added `compile_classes` option to skip compiling classes: 
     33[17:00] rsSix joined the channel 
     34[17:00] <Seldaek> heya 
     35[17:01] <cordoval_> oh 
     36[17:01] <[MA]Pascal> hi 
     37[17:01] <lsmith> which also covers postfixing all compiled PHP files with ".cache": i guess 
     38[17:01] <lsmith> jmikola|w: ping .. 
     39[17:01] <lsmith> Renaming loadUserByUsername(): 
     40[17:01] <johannes_> maybe take another topic first if they are still in a meeting? 
     41[17:02] <lsmith> johannes_: how about removed exit calls: ? :) 
     42[17:02] <johannes_> hm yeah not much i have to add there except that i think we shouldn't have exit calls in core 
     43[17:02] kertz joined the channel 
     44[17:02] phidah joined the channel 
     45[17:02] <lsmith> fabpot: yeah .. i think both sides have stated their POV 
     46[17:03] <lsmith> fabpot: is it a topic you consider for a vote? 
     47[17:03] jTownsend joined the channel 
     48[17:03] <fabpot> which one 
     49[17:03] <lsmith> removed exit calls: 
     50[17:04] <lsmith> you didnt close the PR at least :) 
     51[17:04] <fabpot> if we have the nice exceptions in core, why not; but until then, exit calls will stay 
     52[17:04] Phoop quit IRC (Quit: Page closed) 
     53[17:04] <kriswallsmith> nice exceptions? 
     54[17:04] <johannes_> fabpot: should we add exit calls for other build time exceptions then? 
     55[17:04] webPragmatist joined the channel 
     56[17:04] <fabpot> the PR is not closed because I need to talk with johannes_ about adding the nice exception during build time in the core 
     57[17:05] <lsmith> define "nice exception" please :) 
     58[17:05] <Seldaek> vs 
     59[17:05] webPragmatist quit IRC (Client Quit) 
     60[17:05] <Seldaek> the problem is that this relies on the DebuggerBundle from johannes_ I think 
     61[17:05] <fabpot> Seldaek: correct 
     62[17:05] <[MA]Pascal> Symfony and his core team is glad to announce your page has crashed : reason : ... 
     63[17:06] webPragmatist joined the channel 
     64[17:06] <lsmith> ok .. so there isnt much to discuss further here .. just some work todo 
     65[17:06] bshafs|away is now known as bshafs 
     66[17:07] <lsmith> jmikola|w: please ping when you are back 
     67[17:07] <lsmith> next topic is Form layer feedback: 
     68[17:07] <lsmith> i guess right now i havent heard any complaints about having done the merge 
     69[17:07] <Seldaek> I talked a lot with bernhard a few days ago 
     70[17:07] <lsmith> but there are still a fair number of PR's and tickets open 
     71[17:08] <Seldaek> about the clarity of the Type classes that are atm unreadable imo 
     72[17:08] <Seldaek> (it's an exaggeration, but still) 
     73[17:08] <lsmith> so imho the main task for everybody is to make life as easy as possible for bernhard by reviewing tickets and PR's 
     74[17:08] <Seldaek> he said he'd improve some things, we'll see with time 
     75[17:08] <Herzult> I agree, no doc in code 
     76[17:08] <Herzult> ... 
     77[17:08] <jmikola|w> lsmith: back 
     78[17:08] <jmikola|w> sorry, just finished our morning standup 
     79[17:08] <Seldaek> well docs are one thing, but there is also the fact that you don't know which options are valid and stuff like that 
     80[17:09] <Seldaek> anyway he said he'd work on that, so there's hope ;) 
     81[17:09] <Herzult> :) 
     82[17:09] igorw joined the channel 
     83[17:09] <jmikola|w> kriswallsmith and i started migrating to new form framework, no complaints afaik 
     84[17:10] <jmikola|w> kriswallsmith you did have one problem with getting client data back out of the form, but that's it i think 
     85[17:10] <lsmith> the question is then still what does this mean for our first beta? :-/ 
     86[17:10] <kriswallsmith> yes, i miss the ability to get an array out of the form 
     87[17:11] <fabpot> lsmith: the new new form framework is far better than the old one anyway 
     88[17:11] <lsmith> fabpot: yeah .. 
     89[17:11] <fabpot> it has some bugs, but the old one also have some annoying bugs 
     90[17:12] lsmith nods 
     91[17:12] <lsmith> so we still release beta this week? 
     92[17:12] <fabpot> yes 
     93[17:12] <fabpot> I don't see anything blocking a beta release 
     94[17:12] <Seldaek> ack 
     95[17:13] <lsmith> ok .. i have only played with a little bit .. also no complaints and feels better than before 
     96[17:13] <lsmith> beta = all core features in place, BC breaks only for really really really good reasons 
     97[17:13] <Seldaek> my main issue was with regard to the DateTime field actually 
     98[17:13] <Seldaek> because I think it's totally not Js-calendar friendly 
     99[17:14] <johnwards> Any BC pull requests outstanding? 
     100[17:14] <[MA]Pascal> there is a lot "Incomplete" marked tests 
     101[17:14] <johnwards> BC break pull requests even 
     102[17:14] <johannes_> one likely bc break we will have are annotations 
     103[17:14] <jmikola|w> Are those incomplete tests intl related? 
     104[17:14] <[MA]Pascal> jmikola: Form validation related 
     105[17:15] <fabpot> we can break BC for good reasons, we just need to document everything carefully to ease upgrading 
     106[17:15] lsmith nods 
     107[17:15] jakubzalas joined the channel 
     108[17:16] <lsmith> ok .. so i guess that covers the form topic 
     109[17:16] <lsmith> anyone with some time on their hands please also review some tickets 
     110[17:17] <lsmith> i am sure half of them are already fixed 
     111[17:17] <lsmith> jmikola|w: Renaming loadUserByUsername(): 
     112[17:17] <lsmith> take it away :) 
     113[17:17] <jmikola|w> ok, i was reading 
     114[17:17] <jmikola|w> the topic relates to an old ML discussion about using something like UserId or PrincipalId instead of Username in security component 
     115[17:18] <jmikola|w> i did some digging in security component and found that "username" is used for both the basic security stuff and ACL, so it'd be a big BC change to rename it (perhaps bad with db migrations for some folks) 
     116[17:19] <jmikola|w> but based on reading spring's docs, which johannes_ used as inspiration for a lot of things, i found even they use username mostly everywhere, although they call the "user" himself a principal 
     117[17:19] <lsmith> in other words .. you dont think it makes sense to rename it after all? 
     118[17:19] unknownbliss quit IRC (Ping timeout: 248 seconds) 
     119[17:19] <jmikola|w> well, i know a few of us with apps where you login with email/username or where usernames can change, have had trouble with things in their current state 
     120[17:20] <jmikola|w> so remember-me cookies (non-persistent ones) being based on a changeable username is not ideal 
     121[17:20] <lsmith> jmikola|w: what does spring have there? you mentioned that they do a 2 step process 
     122[17:20] <lsmith> which kinda had me worried because out of the box we already do a query to refresh the user 
     123[17:20] <kriswallsmith> if the current component precludes changing a user's username we should definitely fix that 
     124[17:20] <jmikola|w> lsmith: some confusion on my part - that was the CAS server i believe, when i was working with it last year 
     125[17:21] <lsmith> wouldnt want 2 queries on every request 
     126[17:21] <jmikola|w> when you first login with CAS, it resolves the "username" field to a principal ID (literally the user's ID) 
     127[17:21] <johannes_> kriswallsmith: you can change the username, you just need to make sure to update all references 
     128[17:21] <jmikola|w> makes sense for database-stored users, but probably meaningless for user fixtures or something like ldap backend 
     129[17:22] <jmikola|w> and then CAS's second login step would check the password against the user record identified by that ID 
     130[17:22] <fabpot> I'm lost. Are we talking about just the name, or some feature? 
     131[17:22] <lsmith> fabpot: we moved on to discussion the feature of changing the username 
     132[17:22] <jmikola|w> fabpot: i think a bit of both here 
     133[17:23] <jmikola|w> if we want to have the security token reference a user id instead of a simple username (optionally, perhaps by default for DAO users), that is a functionality change 
     134[17:23] <jmikola|w> the benefit would that any session cookies or remember-me cookies would be far more robust 
     135[17:23] <jmikola|w> and thinking about ACL as well, a user changing their username wouldn't require a massive update to ACL tables 
     136[17:23] <johannes_> jmikola|w: it's one row ;) 
     137[17:24] <jmikola|w> johannes_: oh :) 
     138[17:24] <[MA]Pascal> +1 i'm lost 
     139[17:24] <johannes_> anyway i think the improvement is not bad 
     140[17:24] <lsmith> johannes_: so how would one figure out what is referenced? 
     141[17:25] <lsmith> i guess it requires knowing the configuration and then doing all the necessary updates when updating the username 
     142[17:25] <jmikola|w> fabpot: i just feel this is important because for many sites, changing a username is not unheard of - same with username/email logins 
     143[17:25] <lsmith> aka in something like the FOSUserBundle .. you would kind of need to use an easily customizeable service for this 
     144[17:25] <jmikola|w> so i find the dependence on a "username" to be a weakness 
     145[17:25] <lsmith> so that the dev can add whatever he needs 
     146[17:26] <lsmith> johannes_: ? 
     147[17:26] <jmikola|w> on the same hand, i realize that the field to use as the user identifier needs to be configurable (e.g. username/password fixtures defined inline in the security config) 
     148[17:27] <fabpot> but the username is a unique string associated with your user, that's all 
     149[17:27] <johannes_> lsmith: you mean where it is referenced? 
     150[17:27] <fabpot> by definition, it does not change 
     151[17:27] <lsmith> yes 
     152[17:27] <johannes_> there is no way to predict that 
     153[17:27] <lsmith> fabpot: but it does .. 
     154[17:27] <jmikola|w> fabpot: for many sites, it's unique but it does change - e.g. twitter 
     155[17:27] <fabpot> because you are choosing the wrong field 
     156[17:27] <fabpot> your twitter unique id does not change 
     157[17:28] <jmikola|w> the fact that we use the username for both indexing and display purposes makes it ambiguous 
     158[17:28] lapistano quit IRC (Excess Flood) 
     159[17:28] <jmikola|w> i don't think any new user picking up sf2 would think to have getUsername() (for the AccountInterface) return the user ID 
     160[17:28] lapistano joined the channel 
     161[17:28] dbu quit IRC (Remote host closed the connection) 
     162[17:28] <johannes_> i think the main problem is with the form login where the username could be the email address 
     163[17:28] <fabpot> ok, I understand that the fact we label it username is a problem, but the feature is correct and don't need tobe changed 
     164[17:28] <fabpot> it is only a naming problem 
     165[17:29] <mvrhov> I'd go with UserId 
     166[17:29] <jmikola|w> fabpot: would a feature be necessary to map it to different fields? or is that to be handled by things implementing the AccountInterface 
     167[17:29] <Seldaek> yeah it has to be a unique identifier 
     168[17:29] <Seldaek> unique in space, time, and everything 
     169[17:29] <jmikola|w> mvrhov: i agree UserId would be more well-received than PrincipalId 
     170[17:30] <fabpot> jmikola|w: the Security component does not care about what you choose as the username/id/unique id/principal/whatever we call it 
     171[17:30] <jmikola|w> sorry, /AccountInterface/UserInterface/ - have old sf2 on the brain :) 
     172[17:30] <Seldaek> jmikola|w: the problem is that it's not *really* the userId, since you can use the email or anything. 
     173[17:30] plv quit IRC (Read error: Connection reset by peer) 
     174[17:30] <fabpot> this is your repsonsibility to return a unique string that does not change 
     175[17:30] <Seldaek> jmikola|w: so calling it principal would at least mean that people would RTFM just because it's confusing 
     176[17:30] <jmikola|w> fabpot: i think the login listeners depend on the login form input matching that Id field, though 
     177[17:30] <Seldaek> confusing by design (tm) 
     178[17:31] unknownbliss joined the channel 
     179[17:31] <jmikola|w> that's where an input-to-principal-id resolver comes into play 
     180[17:31] wilmoore quit IRC (Remote host closed the connection) 
     181[17:31] <fabpot> jmikola|w: if we are misusing the username is a totally different problem and probably a bug we need to fix 
     182[17:31] <jmikola|w> because if we just rename loadByUsername to loadByUserId, it means users would probably have to login with their ID 
     183[17:31] lapistano quit IRC (Excess Flood) 
     184[17:31] <jmikola|w> fabpot: i think we're using it for too much as-is - johannes_ if you want to comment 
     185[17:32] <fabpot> anyway, there is no need to reinvent the wheel here as the code is a port from Spring 
     186[17:32] <jmikola|w> it seems to be used for both input/display purposes, and fetching from the DB as the authoritative unique id 
     187[17:32] lapistano joined the channel 
     188[17:32] <fabpot> I have just translated the Java code to PHP and simplified things whenever possible 
     189[17:32] <johannes_> jmikola|w: the best approach is to use a different method for login imo 
     190[17:32] cranberyxl joined the channel 
     191[17:32] <lsmith> ok .. can we wrap this topic up? aka what are the next steps? 
     192[17:33] <jmikola|w> lsmith: i'll discuss with johannes_ offline i guess 
     193[17:33] <lsmith> offline? flying to karlsruhe? :) 
     194[17:33] <jmikola|w> i think a rename of Username to UserId would definitely be in order, though 
     195[17:33] <jmikola|w> hehe 
     196[17:33] <lsmith> ok .. moving on 
     197[17:33] <lsmith> Annotation aliases, namespaces, and conflicts in general: 
     198[17:33] <lsmith> johannes_: ? 
     199[17:34] <johannes_> still the state from my last comment, nothing new there 
     200[17:34] <lsmith> johannes_: i guess its time to decide on the a direction 
     201[17:34] <johannes_> haven't seen beberlei in a while 
     202[17:34] <lsmith> johannes_: i dont think there is a chance in hell that they will do a BC break in Doctrine2 
     203[17:34] <lsmith> doesnt matter how small 
     204[17:35] <lsmith> so either we fork 
     205[17:35] igorw quit IRC (Read error: Connection reset by peer) 
     206[17:35] <johannes_> the class is marked as internal and the class comment explicitly states that no particular care is taken to preserve BC 
     207[17:35] <lsmith> or you guys use different prefixes 
     208[17:35] igorw joined the channel 
     209[17:35] igorw quit IRC (Changing host) 
     210[17:35] igorw joined the channel 
     211[17:35] <johannes_> so why should it not be possible for 2.1? 
     212[17:36] <lsmith> because they maintain BC for 2.x 
     213[17:36] <lsmith> then again .. they have broken BC with the addition of the Exception 
     214[17:36] <kriswallsmith> well, they just broke BC :) 
     215[17:36] <lsmith> right 
     216[17:36] <lsmith> has anyone spoken to beberlei in the last days? 
     217[17:37] <lsmith> he better not be run over by a bus 
     218[17:37] <johannes_> lsmith: as i said haven't seen him lately 
     219[17:37] <lsmith> neither have i 
     220[17:37] <lsmith> maybe extended easter vacation 
     221[17:37] <lsmith> so i guess we cannot move this topic forward either 
     222[17:37] <lsmith> but it kinda sucks not having this resolved before beta 
     223[17:37] <lsmith> given that the extra bundles are part of SE 
     224[17:37] <lsmith> anyway .. last topic 
     225[17:38] <fabpot> I've talked with Jon and he said that the merge is not gonna to happen 
     226[17:38] <fabpot> so, as for the event dispatcher, what about creating our own annotation component? 
     227[17:38] <johannes_> fabpot: did he mention a reason why? 
     228[17:38] <fabpot> johannes_: BC 
     229[17:39] <johannes_> interesting... 
     230[17:39] <fabpot> johannes_: I don't mind forking it and creating a new Symfony component ;) 
     231[17:40] <lsmith> fabpot: yeah .. i think it might actually be a good thing for PHP in general 
     232[17:40] <johannes_> how does lgpl play with mit? 
     233[17:40] <lsmith> to have this in a separate component 
     234[17:40] henrikbjorn quit IRC (Quit: henrikbjorn) 
     235[17:40] <fabpot> johannes_: that's a good question 
     236[17:40] <fabpot> we will have to ask them the authorization to use MIT 
     237[17:40] <lsmith> johannes_: lgpl is more restrictive 
     238[17:40] <lsmith> did we ask about this with the event dispatcher? 
     239[17:41] <fabpot> lsmith: I don't think so 
     240[17:41] <lsmith> doh :-/ 
     241[17:41] <fabpot> I didn't for sure 
     242[17:41] <lsmith> i also dont remember this ever being a topic 
     243[17:41] <fabpot> well, if we need to revert to the old code, that's not a problem ;) 
     244[17:41] <lsmith> fabpot: hrhr 
     245[17:42] <lsmith> ok .. would we consider having LGPL components? 
     246[17:42] <fabpot> lsmith: that makes things complex 
     247[17:42] <lsmith> yes .. though most users will be using Doctrine anyway 
     248[17:42] <jmikola|w> kriswallsmith: on the subject of events, did you have issues with the lack of prefixes? 
     249[17:42] <lsmith> so its not like the LGPL will be possible to evade easily 
     250[17:43] <lsmith> especially since Propel2 also uses Doctrine2 
     251[17:43] <lsmith> kriswallsmith: where is jon? :) 
     252[17:43] <johannes_> we could still create a separate library which only includes the annotation files 
     253[17:43] lsmith nods 
     254[17:43] <jmikola|w> lsmith: just paged him 
     255[17:43] <lsmith> ok .. last topic then 
     256[17:43] henrikbjorn joined the channel 
     257[17:43] henrikbjorn quit IRC (Remote host closed the connection) 
     258[17:43] <lsmith> Cache warming:,, 
     259[17:43] <kriswallsmith> um, tennessee... 
     260[17:44] <johannes_> so is that what we are going to do then? fork the parser? 
     261[17:44] <jonwage> im here 
     262[17:44] krymen quit IRC (Quit: Leaving.) 
     263[17:44] <jonwage> what I would do is not fork it to a new project, but just fork the history and work on a new version 
     264[17:44] skoop quit IRC (Remote host closed the connection) 
     265[17:44] <jonwage> the orm and other doctrine projects cannot update to it right away 
     266[17:44] <jonwage> b/c we guarantee BC for 2.0 for a while 
     267[17:44] weaverryan joined the channel 
     268[17:44] <johannes_> yeah but for 2.1 you think it's possible? 
     269[17:45] <jonwage> but ODM and other unreleased projects can upgrade to it, and ORM 2.x future version can upgrade to it 
     270[17:45] <jonwage> johannes_: maybe ..if not 2.1, another future 2.x version 
     271[17:45] <lsmith> jonwage: aka you could commit it to common master .. 
     272[17:45] <jonwage> well no 
     273[17:45] <jonwage> it would be 2.2 :) 
     274[17:45] <lsmith> huh? why? 
     275[17:45] <jonwage> 2.1 is already in the works 
     276[17:46] <jonwage> we would have to discuss it first 
     277[17:46] <lsmith> but you broke BC in 2.1 
     278[17:46] <jonwage> whether or not Doctrine 2.1 will use it or not 
     279[17:46] <jonwage> i know im just saying 
     280[17:46] <jonwage> its not solely up to me 
     281[17:46] <lsmith> ok 
     282[17:46] <jonwage> its yet another BC break 
     283[17:46] <jonwage> and maybe too big of one 
     284[17:46] <lsmith> so it needs to be discussed further .. can you try and push the topic amongst you? 
     285[17:46] <jonwage> we'll have to discuss that 
     286[17:46] <jonwage> ill talk to Benjamin 
     287[17:46] <johannes_> jonwage: it's only one method that is removed 
     288[17:46] <lsmith> the alternative would be us asking you to letting us relicense the code under the MIT 
     289[17:47] <lsmith> jonwage: which BTW we also need to ask you guys for the EventDispatcher 
     290[17:47] <jonwage> i would rather not split anything as i want to go down the same path as you we're just on different timelines 
     291[17:47] <jonwage> but with git, etc. we can maintain multiple dev versions 
     292[17:47] <jonwage> and not have to split paths 
     293[17:47] <lsmith> well 2.1 ORM will come out around or shortly after Symfony2.0 
     294[17:48] <lsmith> and 2.1 currently breaks Symfony2 SE 
     295[17:48] <lsmith> so from our POV .. it has to be fixed in 2.1 ORM 
     296[17:48] <lsmith> anyway .. now you are in the loop :) 
     297[17:48] <lsmith> so cache warming 
     298[17:49] <lsmith> fabpot: from my experience with cache warming it seemed like we never got it perfectly polished .. i remember you were planning to do some clean ups back when i created that "force flag" PR 
     299[17:49] <lsmith> its not really practical to have to do manual cache warming before running unit tests 
     300[17:49] codecowboy left the channel () 
     301[17:49] <lsmith> see 
     302[17:50] <lsmith> i also kinda never got cache warming to generate proxies for me 
     303[17:51] <lsmith> huhu? :) 
     304[17:54] <Seldaek> that's how it feels to be in a post-apocalyptic zombie movie. 
     305[17:54] noisebleed quit IRC (Remote host closed the connection) 
     306[17:54] <[MA]Pascal> i hope they are all safe 
     307[17:56] <lsmith> :)