Development

IRCLogs20110428

You must first sign up to be able to contribute.

Summary

http://www.doodle.com/i3nfzqdfhhvd9x27#

Added compile_classes option to skip compiling classes

postfixing all compiled PHP files with ".cache"

removed exit calls

Renaming loadUserByUsername()

Form layer feedback

Annotation aliases, namespaces, and conflicts in general

Cache warming

[16:59] <lsmith> so everybody ready to meet today?
[16:59] henrikbjorn quit IRC (Read error: Connection reset by peer)
[16:59] henrikbjorn joined the channel
[16:59] <lsmith> jmikola|w, kriswallsmith, johannes_, henrikbjorn, fabpot, Seldaek, johnwards
[16:59] <johnwards> oh its thursday already
[16:59] rande joined the channel
[16:59] <kriswallsmith> hi
[17:00] Phoop joined the channel
[17:00] <kriswallsmith> opensky is in a meeting, probably another 5 minutes
[17:00] <fabpot> hi all
[17:00] cordoval_ joined the channel
[17:00] codecowboy joined the channel
[17:00] <lsmith> guess the #1 topic is fixed already: Added `compile_classes` option to skip compiling classes: http://bit.ly/fzdz5M
[17:00] rsSix joined the channel
[17:00] <Seldaek> heya
[17:01] <cordoval_> oh
[17:01] <[MA]Pascal> hi
[17:01] <lsmith> which also covers postfixing all compiled PHP files with ".cache": https://github.com/symfony/symfony/pull/633 i guess
[17:01] <lsmith> jmikola|w: ping ..
[17:01] <lsmith> Renaming loadUserByUsername(): http://bit.ly/ef1MRR
[17:01] <johannes_> maybe take another topic first if they are still in a meeting?
[17:02] <lsmith> johannes_: how about removed exit calls: http://bit.ly/hORLKZ ? :)
[17:02] <johannes_> hm yeah not much i have to add there except that i think we shouldn't have exit calls in core
[17:02] kertz joined the channel
[17:02] phidah joined the channel
[17:02] <lsmith> fabpot: yeah .. i think both sides have stated their POV
[17:03] <lsmith> fabpot: is it a topic you consider for a vote?
[17:03] jTownsend joined the channel
[17:03] <fabpot> which one
[17:03] <lsmith> removed exit calls: http://bit.ly/hORLKZ
[17:04] <lsmith> you didnt close the PR at least :)
[17:04] <fabpot> if we have the nice exceptions in core, why not; but until then, exit calls will stay
[17:04] Phoop quit IRC (Quit: Page closed)
[17:04] <kriswallsmith> nice exceptions?
[17:04] <johannes_> fabpot: should we add exit calls for other build time exceptions then?
[17:04] webPragmatist joined the channel
[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
[17:05] <lsmith> define "nice exception" please :)
[17:05] <Seldaek> http://img822.imageshack.us/f/withexception.png/ vs http://img156.imageshack.us/i/withexit.png/
[17:05] webPragmatist quit IRC (Client Quit)
[17:05] <Seldaek> the problem is that this relies on the DebuggerBundle from johannes_ I think
[17:05] <fabpot> Seldaek: correct
[17:05] <[MA]Pascal> Symfony and his core team is glad to announce your page has crashed : reason : ...
[17:06] webPragmatist joined the channel
[17:06] <lsmith> ok .. so there isnt much to discuss further here .. just some work todo
[17:06] bshafs|away is now known as bshafs
[17:07] <lsmith> jmikola|w: please ping when you are back
[17:07] <lsmith> next topic is Form layer feedback: http://bit.ly/e6uP8J
[17:07] <lsmith> i guess right now i havent heard any complaints about having done the merge
[17:07] <Seldaek> I talked a lot with bernhard a few days ago
[17:07] <lsmith> but there are still a fair number of PR's and tickets open
[17:08] <Seldaek> about the clarity of the Type classes that are atm unreadable imo
[17:08] <Seldaek> (it's an exaggeration, but still)
[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
[17:08] <Seldaek> he said he'd improve some things, we'll see with time
[17:08] <Herzult> I agree, no doc in code
[17:08] <Herzult> ...
[17:08] <jmikola|w> lsmith: back
[17:08] <jmikola|w> sorry, just finished our morning standup
[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
[17:09] <Seldaek> anyway he said he'd work on that, so there's hope ;)
[17:09] <Herzult> :)
[17:09] igorw joined the channel
[17:09] <jmikola|w> kriswallsmith and i started migrating to new form framework, no complaints afaik
[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
[17:10] <lsmith> the question is then still what does this mean for our first beta? :-/
[17:10] <kriswallsmith> yes, i miss the ability to get an array out of the form
[17:11] <fabpot> lsmith: the new new form framework is far better than the old one anyway
[17:11] <lsmith> fabpot: yeah ..
[17:11] <fabpot> it has some bugs, but the old one also have some annoying bugs
[17:12] lsmith nods
[17:12] <lsmith> so we still release beta this week?
[17:12] <fabpot> yes
[17:12] <fabpot> I don't see anything blocking a beta release
[17:12] <Seldaek> ack
[17:13] <lsmith> ok .. i have only played with a little bit .. also no complaints and feels better than before
[17:13] <lsmith> beta = all core features in place, BC breaks only for really really really good reasons
[17:13] <Seldaek> my main issue was with regard to the DateTime field actually
[17:13] <Seldaek> because I think it's totally not Js-calendar friendly
[17:14] <johnwards> Any BC pull requests outstanding?
[17:14] <[MA]Pascal> there is a lot "Incomplete" marked tests
[17:14] <johnwards> BC break pull requests even
[17:14] <johannes_> one likely bc break we will have are annotations
[17:14] <jmikola|w> Are those incomplete tests intl related?
[17:14] <[MA]Pascal> jmikola: Form validation related
[17:15] <fabpot> we can break BC for good reasons, we just need to document everything carefully to ease upgrading
[17:15] lsmith nods
[17:15] jakubzalas joined the channel
[17:16] <lsmith> ok .. so i guess that covers the form topic
[17:16] <lsmith> anyone with some time on their hands please also review some tickets http://trac.symfony-project.org/report/24
[17:17] <lsmith> i am sure half of them are already fixed
[17:17] <lsmith> jmikola|w: Renaming loadUserByUsername(): http://bit.ly/ef1MRR
[17:17] <lsmith> take it away :)
[17:17] <jmikola|w> ok, i was reading http://static.springsource.org/spring-security/site/docs/3.1.x/reference/springsecurity-single.html
[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
[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)
[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
[17:19] <lsmith> in other words .. you dont think it makes sense to rename it after all?
[17:19] unknownbliss quit IRC (Ping timeout: 248 seconds)
[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
[17:20] <jmikola|w> so remember-me cookies (non-persistent ones) being based on a changeable username is not ideal
[17:20] <lsmith> jmikola|w: what does spring have there? you mentioned that they do a 2 step process
[17:20] <lsmith> which kinda had me worried because out of the box we already do a query to refresh the user
[17:20] <kriswallsmith> if the current component precludes changing a user's username we should definitely fix that
[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
[17:21] <lsmith> wouldnt want 2 queries on every request
[17:21] <jmikola|w> when you first login with CAS, it resolves the "username" field to a principal ID (literally the user's ID)
[17:21] <johannes_> kriswallsmith: you can change the username, you just need to make sure to update all references
[17:21] <jmikola|w> makes sense for database-stored users, but probably meaningless for user fixtures or something like ldap backend
[17:22] <jmikola|w> and then CAS's second login step would check the password against the user record identified by that ID
[17:22] <fabpot> I'm lost. Are we talking about just the name, or some feature?
[17:22] <lsmith> fabpot: we moved on to discussion the feature of changing the username
[17:22] <jmikola|w> fabpot: i think a bit of both here
[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
[17:23] <jmikola|w> the benefit would that any session cookies or remember-me cookies would be far more robust
[17:23] <jmikola|w> and thinking about ACL as well, a user changing their username wouldn't require a massive update to ACL tables
[17:23] <johannes_> jmikola|w: it's one row ;)
[17:24] <jmikola|w> johannes_: oh :)
[17:24] <[MA]Pascal> +1 i'm lost
[17:24] <johannes_> anyway i think the improvement is not bad
[17:24] <lsmith> johannes_: so how would one figure out what is referenced?
[17:25] <lsmith> i guess it requires knowing the configuration and then doing all the necessary updates when updating the username
[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
[17:25] <lsmith> aka in something like the FOSUserBundle .. you would kind of need to use an easily customizeable service for this
[17:25] <jmikola|w> so i find the dependence on a "username" to be a weakness
[17:25] <lsmith> so that the dev can add whatever he needs
[17:26] <lsmith> johannes_: ?
[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)
[17:27] <fabpot> but the username is a unique string associated with your user, that's all
[17:27] <johannes_> lsmith: you mean where it is referenced?
[17:27] <fabpot> by definition, it does not change
[17:27] <lsmith> yes
[17:27] <johannes_> there is no way to predict that
[17:27] <lsmith> fabpot: but it does ..
[17:27] <jmikola|w> fabpot: for many sites, it's unique but it does change - e.g. twitter
[17:27] <fabpot> because you are choosing the wrong field
[17:27] <fabpot> your twitter unique id does not change
[17:28] <jmikola|w> the fact that we use the username for both indexing and display purposes makes it ambiguous
[17:28] lapistano quit IRC (Excess Flood)
[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
[17:28] lapistano joined the channel
[17:28] dbu quit IRC (Remote host closed the connection)
[17:28] <johannes_> i think the main problem is with the form login where the username could be the email address
[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
[17:28] <fabpot> it is only a naming problem
[17:29] <mvrhov> I'd go with UserId
[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
[17:29] <Seldaek> yeah it has to be a unique identifier
[17:29] <Seldaek> unique in space, time, and everything
[17:29] <jmikola|w> mvrhov: i agree UserId would be more well-received than PrincipalId
[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
[17:30] <jmikola|w> sorry, /AccountInterface/UserInterface/ - have old sf2 on the brain :)
[17:30] <Seldaek> jmikola|w: the problem is that it's not *really* the userId, since you can use the email or anything.
[17:30] plv quit IRC (Read error: Connection reset by peer)
[17:30] <fabpot> this is your repsonsibility to return a unique string that does not change
[17:30] <Seldaek> jmikola|w: so calling it principal would at least mean that people would RTFM just because it's confusing
[17:30] <jmikola|w> fabpot: i think the login listeners depend on the login form input matching that Id field, though
[17:30] <Seldaek> confusing by design (tm)
[17:31] unknownbliss joined the channel
[17:31] <jmikola|w> that's where an input-to-principal-id resolver comes into play
[17:31] wilmoore quit IRC (Remote host closed the connection)
[17:31] <fabpot> jmikola|w: if we are misusing the username is a totally different problem and probably a bug we need to fix
[17:31] <jmikola|w> because if we just rename loadByUsername to loadByUserId, it means users would probably have to login with their ID
[17:31] lapistano quit IRC (Excess Flood)
[17:31] <jmikola|w> fabpot: i think we're using it for too much as-is - johannes_ if you want to comment
[17:32] <fabpot> anyway, there is no need to reinvent the wheel here as the code is a port from Spring
[17:32] <jmikola|w> it seems to be used for both input/display purposes, and fetching from the DB as the authoritative unique id
[17:32] lapistano joined the channel
[17:32] <fabpot> I have just translated the Java code to PHP and simplified things whenever possible
[17:32] <johannes_> jmikola|w: the best approach is to use a different method for login imo
[17:32] cranberyxl joined the channel
[17:32] <lsmith> ok .. can we wrap this topic up? aka what are the next steps?
[17:33] <jmikola|w> lsmith: i'll discuss with johannes_ offline i guess
[17:33] <lsmith> offline? flying to karlsruhe? :)
[17:33] <jmikola|w> i think a rename of Username to UserId would definitely be in order, though
[17:33] <jmikola|w> hehe
[17:33] <lsmith> ok .. moving on
[17:33] <lsmith> Annotation aliases, namespaces, and conflicts in general: http://bit.ly/ez4dpM
[17:33] <lsmith> johannes_: ?
[17:34] <johannes_> still the state from my last comment, nothing new there
[17:34] <lsmith> johannes_: i guess its time to decide on the a direction
[17:34] <johannes_> haven't seen beberlei in a while
[17:34] <lsmith> johannes_: i dont think there is a chance in hell that they will do a BC break in Doctrine2
[17:34] <lsmith> doesnt matter how small
[17:35] <lsmith> so either we fork
[17:35] igorw quit IRC (Read error: Connection reset by peer)
[17:35] <johannes_> the class is marked as internal and the class comment explicitly states that no particular care is taken to preserve BC
[17:35] <lsmith> or you guys use different prefixes
[17:35] igorw joined the channel
[17:35] igorw quit IRC (Changing host)
[17:35] igorw joined the channel
[17:35] <johannes_> so why should it not be possible for 2.1?
[17:36] <lsmith> because they maintain BC for 2.x
[17:36] <lsmith> then again .. they have broken BC with the addition of the Exception
[17:36] <kriswallsmith> well, they just broke BC :)
[17:36] <lsmith> right
[17:36] <lsmith> has anyone spoken to beberlei in the last days?
[17:37] <lsmith> he better not be run over by a bus
[17:37] <johannes_> lsmith: as i said haven't seen him lately
[17:37] <lsmith> neither have i
[17:37] <lsmith> maybe extended easter vacation
[17:37] <lsmith> so i guess we cannot move this topic forward either
[17:37] <lsmith> but it kinda sucks not having this resolved before beta
[17:37] <lsmith> given that the extra bundles are part of SE
[17:37] <lsmith> anyway .. last topic
[17:38] <fabpot> I've talked with Jon and he said that the merge is not gonna to happen
[17:38] <fabpot> so, as for the event dispatcher, what about creating our own annotation component?
[17:38] <johannes_> fabpot: did he mention a reason why?
[17:38] <fabpot> johannes_: BC
[17:39] <johannes_> interesting...
[17:39] <fabpot> johannes_: I don't mind forking it and creating a new Symfony component ;)
[17:40] <lsmith> fabpot: yeah .. i think it might actually be a good thing for PHP in general
[17:40] <johannes_> how does lgpl play with mit?
[17:40] <lsmith> to have this in a separate component
[17:40] henrikbjorn quit IRC (Quit: henrikbjorn)
[17:40] <fabpot> johannes_: that's a good question
[17:40] <fabpot> we will have to ask them the authorization to use MIT
[17:40] <lsmith> johannes_: lgpl is more restrictive
[17:40] <lsmith> did we ask about this with the event dispatcher?
[17:41] <fabpot> lsmith: I don't think so
[17:41] <lsmith> doh :-/
[17:41] <fabpot> I didn't for sure
[17:41] <lsmith> i also dont remember this ever being a topic
[17:41] <fabpot> well, if we need to revert to the old code, that's not a problem ;)
[17:41] <lsmith> fabpot: hrhr
[17:42] <lsmith> ok .. would we consider having LGPL components?
[17:42] <fabpot> lsmith: that makes things complex
[17:42] <lsmith> yes .. though most users will be using Doctrine anyway
[17:42] <jmikola|w> kriswallsmith: on the subject of events, did you have issues with the lack of prefixes?
[17:42] <lsmith> so its not like the LGPL will be possible to evade easily
[17:43] <lsmith> especially since Propel2 also uses Doctrine2
[17:43] <lsmith> kriswallsmith: where is jon? :)
[17:43] <johannes_> we could still create a separate library which only includes the annotation files
[17:43] lsmith nods
[17:43] <jmikola|w> lsmith: just paged him
[17:43] <lsmith> ok .. last topic then
[17:43] henrikbjorn joined the channel
[17:43] henrikbjorn quit IRC (Remote host closed the connection)
[17:43] <lsmith> Cache warming: http://bit.ly/hG6p9X, http://bit.ly/gTMW8Y, http://bit.ly/dXWDHp
[17:43] <kriswallsmith> um, tennessee...
[17:44] <johannes_> so is that what we are going to do then? fork the parser?
[17:44] <jonwage> im here
[17:44] krymen quit IRC (Quit: Leaving.)
[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
[17:44] skoop quit IRC (Remote host closed the connection)
[17:44] <jonwage> the orm and other doctrine projects cannot update to it right away
[17:44] <jonwage> b/c we guarantee BC for 2.0 for a while
[17:44] weaverryan joined the channel
[17:44] <johannes_> yeah but for 2.1 you think it's possible?
[17:45] <jonwage> but ODM and other unreleased projects can upgrade to it, and ORM 2.x future version can upgrade to it
[17:45] <jonwage> johannes_: maybe ..if not 2.1, another future 2.x version
[17:45] <lsmith> jonwage: aka you could commit it to common master ..
[17:45] <jonwage> well no
[17:45] <jonwage> it would be 2.2 :)
[17:45] <lsmith> huh? why?
[17:45] <jonwage> 2.1 is already in the works
[17:46] <jonwage> we would have to discuss it first
[17:46] <lsmith> but you broke BC in 2.1
[17:46] <jonwage> whether or not Doctrine 2.1 will use it or not
[17:46] <jonwage> i know im just saying
[17:46] <jonwage> its not solely up to me
[17:46] <lsmith> ok
[17:46] <jonwage> its yet another BC break
[17:46] <jonwage> and maybe too big of one
[17:46] <lsmith> so it needs to be discussed further .. can you try and push the topic amongst you?
[17:46] <jonwage> we'll have to discuss that
[17:46] <jonwage> ill talk to Benjamin
[17:46] <johannes_> jonwage: it's only one method that is removed
[17:46] <lsmith> the alternative would be us asking you to letting us relicense the code under the MIT
[17:47] <lsmith> jonwage: which BTW we also need to ask you guys for the EventDispatcher
[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
[17:47] <jonwage> but with git, etc. we can maintain multiple dev versions
[17:47] <jonwage> and not have to split paths
[17:47] <lsmith> well 2.1 ORM will come out around or shortly after Symfony2.0
[17:48] <lsmith> and 2.1 currently breaks Symfony2 SE
[17:48] <lsmith> so from our POV .. it has to be fixed in 2.1 ORM
[17:48] <lsmith> anyway .. now you are in the loop :)
[17:48] <lsmith> so cache warming
[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
[17:49] <lsmith> its not really practical to have to do manual cache warming before running unit tests
[17:49] codecowboy left the channel ()
[17:49] <lsmith> see https://github.com/fabpot/symfony/pull/618
[17:50] <lsmith> i also kinda never got cache warming to generate proxies for me
[17:51] <lsmith> huhu? :)
[17:54] <Seldaek> that's how it feels to be in a post-apocalyptic zombie movie.
[17:54] noisebleed quit IRC (Remote host closed the connection)
[17:54] <[MA]Pascal> i hope they are all safe
[17:56] <lsmith> :)