RFC: Adding DI to Forms - experimental refactoring
Configuration class improvements
Whats missing for RC1?
RFC: support for 405 Method Not Allowed responses
[17:00] <lsmith> ok .. lets go
[17:00] → [MA]Pascal joined the channel.
[17:00] <lsmith> bschussek: ping
[17:01] <[MA]Pascal> hi guys
[17:01] → kertz joined the channel.
[17:01] <bschussek> lsmith: pong
[17:01] <lsmith> RFC: Adding DI to Forms - experimental refactoring: http://bit.ly/fuYIUd
[17:02] <lsmith> your turn bschussek
[17:02] <lsmith> where are we .. when will a pull be ready
[17:02] <lsmith> how can we help
[17:02] <lsmith> etc
[17:02] <bschussek> ok
[17:02] <bschussek> we currently have a new architecture
[17:03] <bschussek> but things need to be cleaned up
[17:03] <bschussek> that's what I'm working on right now
[17:03] <lsmith> vicb: your topic is next btw
[17:03] <bschussek> beberlei is working on a PhpTheme and helps me with feedback
[17:03] <vicb> ok, getting prepared
[17:03] <bschussek> I fear there's currently not much more anyone can do
[17:03] <lsmith> ok .. is there a reason for waiting to merge this?
[17:03] <lsmith> i mean its a gigantic change
[17:04] <lsmith> and everybody is sitting on their hands waiting for it to appear
[17:04] <bschussek> lsmith: yes. Some small features are currently broken
[17:04] <lsmith> its the single biggest change before RC1
[17:04] → everzet joined the channel.
[17:04] <bschussek> I have to fix these before merging
[17:04] <bschussek> and in order to fix these, things need to be cleaned up
[17:04] <mvrhov> lsmith, you are forgeting about the Event change
[17:05] <lsmith> basically we cannot release an RC1 earlier than 1 week .. likely 2 weeks after this has been merged
[17:05] <bschussek> mvrhov is right. The event change was a big requirement for this merge
[17:05] ← Nenuial left IRC. (Read error: Operation timed out)
[17:05] <johnwards> bschussek: The event code is in the branch is that complete and will that be in the same pull request?
[17:05] <bschussek> as it looks, the event code is merged today
[17:05] <lsmith> mvrhov: the event hcnage isnt "big"
[17:05] <bschussek> lsmith: no, but Form relies on it
[17:06] <lsmith> fabpot: ping
[17:07] → Nenuial joined the channel.
[17:07] <fabpot> I think I won't be able to merge the new event system today
[17:07] <fabpot> I'm still reviewing the patch, and discussing some renaming with bschussek
[17:08] <fabpot> I'm also converting the FrameworkExtraBundle to the new system to have a better feeling for it
[17:08] <fabpot> so, hopefully, this will land into master tomorrow
[17:08] <fabpot> lsmith: and yes, the new form is the last thing we are waiting for
[17:08] <fabpot> by the way, it's not clear that it will land or not right now
[17:09] <fabpot> I have raised some concerns to bschussek, and they need to be fixed before we can consider merging it
[17:09] <johnwards> bschussek: I'm using your branch. Do you need help? Or are you happy for me to write tests?
[17:09] ← julbou left IRC. (Ping timeout: 250 seconds)
[17:09] <bschussek> johnwards: you can write tests
[17:09] → DeluxZ joined the channel.
[17:09] <bschussek> there are some tests missing for the value transformers, for example
[17:09] <lsmith> maybe it would be better to create a WIP PR
[17:09] <lsmith> so that we have a centralized place for feedback?
[17:10] <bschussek> lsmith: talking about forms or events?
[17:10] <Stof> events still have a PR so I guess forms
[17:10] <lsmith> forms
[17:10] ← [MA]Pascal left IRC. (Quit: [MA]Pascal)
[17:11] <lsmith> ok .. is there anything else for this topic?
[17:11] <johnwards> timeline?
[17:11] <bschussek> lsmith: I'm not sure it already makes sense at this point. Most people will complain about things that are going to be changed anyway
[17:12] → [MA]Pascal joined the channel.
[17:12] <lsmith> seems like the opensky guys are sleeping again .. Seldaek too?
[17:12] <bschussek> i.e., small convenience things, order of arguments, such stuff
[17:12] <Seldaek> lsmith: nah
[17:12] <Seldaek> lsmith: just don't want to enter the forms debate:p
[17:12] <lsmith> bschussek: do you have an ETA?
[17:12] → ahilles107 joined the channel.
[17:13] <bschussek> hopefully end of next week
[17:13] <lsmith> ok :-/
[17:13] <fabpot> bschussek: I think we cannot wait much more than that
[17:13] <Seldaek> bschussek: are you *sure* you can't delegate some stuff?
[17:13] <bschussek> Seldaek: I can delegate testing, I can't delegate the cleanup because then I'd spent more time explaining than actually working
[17:14] <bschussek> fabpot: ok
[17:14] <Seldaek> I somehow doubt that but ok..
[17:14] <johnwards> I can also chip in with little fixes as I'm actually using the code to develop
[17:14] <rande> why we cannot wait more ?
[17:14] <lsmith> meaning we kind of need a "final" version by mid next week to review
[17:14] <lsmith> so that we can vote on if we want the change in during the next meeting
[17:14] <fabpot> rande: because people expect us to release something, people expect us to have documentation, ...
[17:15] <lsmith> work out last details and then merge over the weekend
[17:15] <fabpot> if we don't move fast, we will loose the momentum
[17:15] <weaverryan> I agree - time to close things off
[17:15] <lsmith> can someone else take over the event stuff in case there is still stuff to do?
[17:15] <beberlei> now here, customer
[17:15] <fabpot> lsmith: I think it won't need much work, so I will
[17:15] <lsmith> is there some austrian that can take over bschussek's laundry and house work chores? :)
[17:16] <johnwards> Is the form refactoring not a decided on thing? Especially after the talk at sflive2011 it seemed like thats its a definite in?
[17:17] <beberlei> generally people are concerned by the DICish look, but its perfectly self-sustainable, and even easy so, we just have to get the convenience for it done, first integration into framework is important
[17:17] <fabpot> johnwards: no, it's not. As I said, I raised some concerns and they need to be fixed. But bschussek agreed on them and they should be fixable
[17:17] <lsmith> ok
[17:17] <lsmith> so lets move on
[17:17] <lsmith> vicb: Configuration class improvements: http://bit.ly/gZ2giC, http://bit.ly/fe4DC8
[17:17] <lsmith> johanness, Stof are you guys around too?
[17:17] <Stof> I'm here
[17:18] <vicb> The main idea was to be able to easily generate docs & xsd
[17:18] <kertz> Is there a todo list ordered on priority with who's working on what?
[17:18] <vicb> It also comes with some other benefits detailed in the PR
[17:18] <vicb> Johannes has agreed on integrating the changes
[17:19] <lsmith> the PR is the second link .. aka https://github.com/symfony/symfony/pull/303
[17:19] <vicb> So I think they should be integrated asap
[17:19] <vicb> There is one thing left to be done:
[17:19] <fabpot> I can merge it when it's ready and if we all agree
[17:19] <fabpot> I'm +1
[17:19] <vicb> test the config for the security component
[17:20] <jmikola|w> i didn't understand what adding a children() block accomplished, but the migration looks very painless and i trust your judgement
[17:20] <vicb> i don't have a proper setup to exercize all the options
[17:20] <Stof> I'm +1 too
[17:20] <jmikola|w> weaverryan: did you give this a look? i think you were playing with some config console task?
[17:20] <vicb> Any volunteer to help with security ?
[17:20] → seb_m joined the channel.
[17:20] <Stof> vicb: you will need to rebase it because there is some changes in the configuration classes since you did it
[17:20] → julbou joined the channel.
[17:20] <vicb> I have a rebased version I can push
[17:20] <Stof> at least SwiftmailerBundle
[17:21] <jmikola|w> was FrameworkBundle already migrated?
[17:21] <fabpot> vicb: you can probably test with the new Symfony SE, as we have a simple security config
[17:21] <weaverryan> jmikola|w: I haven't in detail, but I don't think it'll affect anything (and if so, it'll be positive)
[17:21] → henrikbjorn joined the channel.
[17:21] <vicb> All the core extension have been migrated
[17:21] <Seldaek> jmikola|w: aye, didn't really get the children() calls, but it's no big deal
[17:22] <Stof> jmikola|w: all bundle are migrated in the PR
[17:22] <vicb> i can try SE but if anyone has a setup with security enabled that should really be a matter of pulling the branch and testing
[17:22] <Stof> vicb: the latest SE ahs the security enabled
[17:23] <jmikola|w> kriswallsmith: i think we can test this out with opensky once it's in core :)
[17:23] <jmikola|w> we have a rather gigantic security configuration
[17:23] <vicb> I'll push my branch right after the meeting then
[17:24] <lsmith> ok sounds good
[17:24] <lsmith> next topic then i guess
[17:24] <vicb> Then there will be some work to do to generate the docs but is this the topic ?
[17:24] <vicb> ok
[17:25] <jmikola|w> vicb: that's a separate task, and likely something we can work on after RC1 is released :)
[17:25] <lsmith> right
[17:25] <jmikola|w> no BC concerns
[17:25] <lsmith> so next topic is Whats missing for RC1?
[17:25] <vicb> agree
[17:25] <lsmith> obviously the form stuff
[17:26] <lsmith> anything else critical that is missing?
[17:26] <johanness> not so sure...
[17:26] <johanness> if we don't plan for having doc generation, it might not work
[17:26] <fabpot> as far as the code is concerned, I think it's mainly the form stuff
[17:26] <fabpot> but we also need to think about the service names before going RC1
[17:27] <fabpot> besides some inconsistencies, getting the Doctrine manager for instance is quite verbose
[17:27] <vicb> I think defining an interface for the Configuration clss might be enough ?
[17:27] <jmikola|w> johanness: i think we needed a way to add doc comments (since closures can't be turned into descriptive text easily), but other than that, i don't see problems inferring documentation based on the config structure (can discuss later if you like)
[17:27] <fabpot> so, we need to list all service names and see if we can come up with better names
[17:27] <everzet> fabpot: +1. Some service names is really to verbsoe
[17:27] → old_sound joined the channel.
[17:28] <everzet> *are really too verbsoe
[17:28] <everzet> *verbose
[17:28] <jmikola|w> fabpot: should we have a best practice for naming services? i think a leading vendor name certainly helps, so perhaps the default EM could be doctrine.entity_manager
[17:28] <lsmith> fabpot: what about bridges?
[17:28] <lsmith> and the other stuff we discussed in paris
[17:28] <lsmith> http://trac.symfony-project.org/wiki/SfliveParisDevMeeting
[17:29] <lsmith> "default scope"
[17:29] <fabpot> lsmith: I know we have some other things, I just wanted to talk about things we don't have yet on our radar
[17:29] <lsmith> k
[17:30] <[MA]Pascal> i think we need more tests (core + bundles + SE)
[17:30] <johnwards> Do we have a list of all the things on the radar? Is that the dev meeting notes?
[17:30] <jmikola|w> johnwards: i think the paris meeting notes lsmith posted above + what we're discussing here should be it
[17:30] <beberlei> i agree pascal, i found some stuff that is entirely untested
[17:31] <beberlei> mostly stuff thta is only good to test on a functional level, but still missing
[17:31] <johanness> btw, do we have functional tests atm?
[17:31] <beberlei> what i fear is that with all the unittests we dont see when larger stuff with the DIC config changes
[17:31] <beberlei> unittests wont see this at all
[17:32] <johanness> especially for the security listeners functional tests would make sense imo
[17:32] <jmikola|w> when i was dabbling with frameworkbundle, i wrote its extension test functionally (reads config files)... but i know not all the extensions do that - some are strictly unit
[17:32] <beberlei> maybe we even need some example applications
[17:32] <beberlei> yah security extension is probably the best example where a full app test would probably be nececssary
[17:33] <jmikola|w> beberlei: perhaps that could be achieved by making some webtests in the sandbox application, that load some controllers - security could be added there, too
[17:33] <jmikola|w> i'm not sure where such tests would fit in within the main symfony repository
[17:33] <beberlei> you dont run the sandbox tests in conjunction with the symfony tests
[17:33] <beberlei> tests/apps/demo1/*
[17:33] <beberlei> you obviously dont need the vendor in there, you already have that
[17:34] ← old_sound left IRC. (Quit: old_sound)
[17:34] ← carlossg_00 left IRC. (Quit: Leaving)
[17:34] <[MA]Pascal> +1 for a hardened distribution
[17:35] <everzet> separate test distribution?
[17:35] <everzet> +1 then
[17:35] <beberlei> nah, nobody uses that
[17:35] <fabpot> as the directory structure for an app is quite flexible, it can be just a bunch of configuration files and some controllers/templates
[17:35] <pgodel_work> everzet: I was thinking exactly the same
[17:35] <fabpot> all tests should be in the main repository
[17:36] <everzet> then we can do something, like i dit in BehatBundle
[17:36] <beberlei> whats the problem with putting together very small apps in the test folder? i cant see a downside
[17:36] <everzet> create test controllers/views under the Test namespace in bundle itself
[17:36] <johnwards> +1 on the BehatBundle tests. Works well
[17:37] <jmikola|w> beberlei: putting test apps in a test folder sounds fine, esp if the directory structure is flexible (we can take shortcuts :)
[17:38] <fabpot> beberlei: +1
[17:38] <fabpot> jmikola|w: yes, we will take shortcuts
[17:38] <Stof> +1
[17:38] <jmikola|w> fabpot: are we going to move tests into the actual components?
[17:38] <fabpot> jmikola|w: no
[17:39] <everzet> jmikola|w: no
[17:39] ← julbou left IRC. (Quit: julbou)
[17:39] <everzet> jmikola|w: you don't need functional web tests inside components. Only in the bundles
[17:39] <jmikola|w> everzet: i was talking about unit tests
[17:39] → dbenjamin joined the channel.
[17:39] <everzet> jmikola|w: ah, you're talking bout unit-tests. sorry :-/
[17:40] <lsmith> ok .. anything else that is missing for RC1 that might not be on our radar yet?
[17:40] <jmikola|w> yeah, looking at https://github.com/symfony/Templating i realize there are no tests there
[17:40] <lsmith> ok moving on then
[17:40] <lsmith> Seldaek: Adding MonologBundle in the core: http://bit.ly/eryhFl
[17:41] <Seldaek> well, there isn't much to say I guess everyone knows what it's about
[17:41] <Seldaek> all we need is a decision
[17:42] <[MA]Pascal> everzet: yeah i was talking about functional tests too ..
[17:42] <Seldaek> there are a few advantages (FingersCrossedHandler, flexibility of development - ever tried contributing to ZF?, multiple logging channels - which could allow filtering of this or that log channel easily in the profiler)
[17:43] <Seldaek> afaik there is no drawback and if there's one I'm pretty sure we can work on fixing it
[17:43] <jmikola|w> Seldaek: would this work like assetic? bundle in core and the main library in your repo?
[17:44] <pgodel_work> log rotation is pretty nice too
[17:44] <Seldaek> jmikola|w: yes, it's also API compatible, so there really is not much of a change required in core to support either logging library
[17:44] <jmikola|w> i stuff you presented during unconf at sflive looked good, so i have no objections
[17:44] <Seldaek> pgodel_work: aye, that's not there yet though, but definitely planned
[17:44] <jmikola|w> things like fingerscrossed defintely could make debugging sf apps easier (when you tail logs)
[17:44] <beberlei> can someone explain this finger crossing?
[17:44] <jmikola|w> for anyone unaware, it basically doesn't log anything unless a serious error pops up
[17:45] <jmikola|w> then you get the full log (all levels)
[17:45] <Stof> jmikola|w: the current implementation also uses a separate vendor for logs
[17:45] <lsmith> it should be noted that switching from monolog to zend\log is trivial
[17:45] <lsmith> however the reason to have monolog in core is that the internal services would then be tagged properly out of the box
[17:46] <beberlei> tagged as what?
[17:46] <johnwards> I have yet to see a negative opinion on including this. Anyone wish to play devils advocate?
[17:46] <beberlei> ah you mean vs not tagged if monolog is not in core
[17:46] <snc_hw> beberlei: logging channels
[17:46] <Seldaek> beberlei: the tags define the logging channel that every service using the logger will use
[17:47] <[MA]Pascal> Seldaek: i rly like it, my only concern so far is the CRAP/Cyclomatic Complexity score of some methods (foreach->foreach->if->foreach->if in LoggerChannelPass e.g.)
[17:47] <Seldaek> beberlei: that way, all db stuff is tagged as "db" channel and can be filtered out easily (that's just an example..)
[17:47] <johanness> Seldaek, one logger can log only one channel?
[17:47] → dustinwhittle joined the channel.
[17:47] <fabpot> johnwards: I can
[17:47] <Stof> [MA]Pascal: this is needed because of the way the tag work. you need to loop over the tags of the tagged services
[17:47] <johnwards> I thought that might be the case fabpot ;)
[17:48] <fabpot> that's yet another thing that comes late in the game
[17:48] <Stof> but it is only done at compile time
[17:48] <Seldaek> johanness: yes, the logger instance *is* the channel, but you could extend it and support multiple channels if you really have a use case
[17:48] <fabpot> and nothings comes for free
[17:48] <fabpot> so, I will only consider adding it to the core when full documentation and tests are available
[17:48] <pgodel_work> fair enough
[17:49] ← ornicar left IRC. (Ping timeout: 252 seconds)
[17:49] <Seldaek> fabpot: might as well have told me that two weeks ago when I asked though.. I'm not avail this weekend :/
[17:49] <Stof> I could advocate the opposite by saying Assetic is in the core without doc :)
[17:49] <Seldaek> yeah, also there are really not that many docs required, it's so trivial to use
[17:49] <lsmith> i agree with requiring tests
[17:49] <Stof> (and ZendBundle is not documented either)
[17:50] <Seldaek> and from the end user perspective, it acts just like Zend\Log
[17:50] <[MA]Pascal> + 1 for no need to doc but +1 for full coverage unittest
[17:50] <Seldaek> you can't really distinguish them
[17:50] <lsmith> but doc's is something that should be committed to be provided before a stable release
[17:50] <Seldaek> monolog itself is tested
[17:50] <fabpot> Seldaek: I'm just a bit upset right now because I've already spent 2 hours debugging the new event system and fixing bugs
[17:51] <Seldaek> fabpot: well, I can understand, but this isn't the same kind of core change, the Logger interface is clearly defined and respected..
[17:51] ← dbu left IRC. (Remote host closed the connection)
[17:51] <Seldaek> fabpot: the PR is probably out of date compared to core though, but I can fix that easily if we agree to merge it
[17:51] <fabpot> Seldaek: I know the story. Don't be worry, this is a very simple change, ...
[17:51] <Stof> the compiler pass is tested so the only missing test is the DI extension. I can write them this evening
[17:51] <[MA]Pascal> Seldaek: so testing the bundle should be trivial
[17:51] ← dustinwhittle left IRC. (Client Quit)
[17:52] <fabpot> again, someone ask for a devil advocate, that's what I'm trying to do now ;)
[17:52] <Seldaek> [MA]Pascal: yes, see Stof's comment
[17:52] <lsmith> ok .. so then lets vote?
[17:52] <Seldaek> fabpot: yeah well, there is devil's advocate and abusive devil :p
[17:52] <Stof> and I can rebase it before that
[17:52] <Stof> +1
[17:53] <Seldaek> +1 (in case it wasn't clear..)
[17:53] <fabpot> whatever the vote result will be, it must be mergeable by the end of next week
[17:53] <rande> +1
[17:53] <johnwards> +1
[17:53] <vicb> +1
[17:53] <kertz> +1
[17:53] <Seldaek> fabpot: no problem
[17:53] <snc_hw> +1
[17:53] <igorw> +1
[17:53] <[MA]Pascal> +1
[17:53] <Stof> I can do it mergeable this evening
[17:53] <kriswallsmith> -0
[17:53] <rande> ;)
[17:54] <[MA]Pascal> kriswallsmith: bad boy
[17:54] <lsmith> ok next topic
[17:54] <Seldaek> kriswallsmith: disagreeing for the sake of it? :p
[17:54] <fabpot> and of course, someone (Seldaek?) should be committed to maintain monolog for the forseable
[17:54] <naderman> -?
[17:54] <lsmith> RFC: support for 405 Method Not Allowed responses: http://bit.ly/fCwcJC
[17:54] <lsmith> kriswallsmith:
[17:54] <lsmith> go fast .. :)
[17:54] <kriswallsmith> i think we should support 405 responses
[17:54] <Seldaek> fabpot: sure, I don't plan on dropping Sf2 any time soon :)
[17:54] <kriswallsmith> if the pathinfo points to a known resource but the method is not supported
[17:55] <kriswallsmith> that's what this patch is
[17:55] <[MA]Pascal> Seldaek: i will add Monolog to our jenkins ci platform today
[17:55] <kriswallsmith> it changes a few parts of the router interface
[17:55] <henrikbjorn> i would like to raise something with this, we need to find a uniformed way of handling response codes, exceptions or returning response objects (NotFound 404 and other non successful responses)
[17:55] <kriswallsmith> the match method throws an exception instead of return false
[17:55] <lsmith> henrikbjorn: http://webmachine.basho.com/diagram.html
[17:55] <fabpot> I'm +1 for this PR, I think it's far cleaner than the current code
[17:55] <kriswallsmith> henrikbjorn: i've tried to do that here
[17:56] → dustinwhittle joined the channel.
[17:56] <kriswallsmith> the router throwing an exception rather than returning false is the main change
[17:56] <fabpot> but the patch is not mergeable yet as the Apache Dumper is not updated yet
[17:56] <kriswallsmith> right
[17:56] <henrikbjorn> also the redirect is a response that should be an exception then
[17:57] <kriswallsmith> that's outside the scope of this pr
[17:57] ← seb_m left the channel.
[17:57] <kriswallsmith> so, any objections?
[17:57] <kriswallsmith> not sure what there is to discuss...
[17:57] <kriswallsmith> it's just a cool thing to say we support :)
[17:57] <henrikbjorn> yes i know but still a a concern that the handling of theese are inconsistent.
[17:57] <johanness> i also like to have a general concept for exception vs response
[17:57] <henrikbjorn> +1 from me
[17:58] <henrikbjorn> Maybe the rule should be exceptions for everything not 2xx ?
[17:58] <kriswallsmith> let's discuss exception v. response next week
[17:59] <kriswallsmith> i'll assuming we're overwhelmingly in favor of my PR :)
[17:59] <henrikbjorn> sure
[17:59] <Seldaek> henrikbjorn: I don't think 3xx wararnt an exception
[17:59] <kriswallsmith> OT
[17:59] <Seldaek> having exceptions for the errors makes sense
[17:59] <kertz> Seldaek: +1
[17:59] <kriswallsmith> alright, meeting adjourned?
[18:00] <Seldaek> kriswallsmith: other than that, on principle I agreee this has to be supported
[18:00] <henrikbjorn> Seldaek: that might be right, but it would skip the response process because it will be catched early
[18:00] <fabpot> so, let's open a discussion on exception vs response on the dev mailing-list (no need to wait for next week)
[18:00] <lsmith> yes .. so yeah .. meeting over