Hello, On Sat, Jan 18, 2014 at 11:08 PM, Stefan Schmidt <[email protected]> wrote: > On Fri, 2014-01-17 at 11:08, Daniel Kolesa wrote: >> quaker pushed a commit to branch master. >> >> http://git.enlightenment.org/core/efl.git/commit/?id=6db16915951af61d28f62f4de7c14c76e4303f18 >> >> commit 6db16915951af61d28f62f4de7c14c76e4303f18 >> Author: Daniel Kolesa <[email protected]> >> Date: Fri Jan 17 19:07:59 2014 +0000 >> >> edje: use luajit by default, if you want lua use --enable-lua-old > > I'm unhappy with this commit. > > Where and when was it decided to switch on a new default dependency? > For instance jenkins is broken for all efl related builds now because > nobody told me or beber in advance that luajit will be needed from now > on. And as I have a busy weekend I will only be anle to look into this > from Monday. I think something like this should be communicated before > the actual change hits the repo.
I second you on that. The fact that the bot got broken is something really bad and we should avoid that. Beber and Stefan are doing a great job at mainting it and are very responsive. It is good practice to at least talk with them before. > Second, 90% of this patch is indent changes not related to enable > luajit. Meaning there is tons of noise in this commit before I can > actually see what did change. And yes, I was looking through this > commit to see what it did to break the build. I have seeen more and more patch coming in where people mix indentation change and code. I was going to complain about Hermet recent change on ecore_evas, but I will just complain here for both of them. Please have to separated patch if you really want to do a space fix. I do think it is wrong to modify code to fix code indentation or you accept to take responsibility for any bug that will be related to those line. Meaning you should be able to understand and explain every line you modify for just space. I tend to use git blame to ask people about what a code does, and if your patch was just about changing space, that is going to be more difficult to understand who as the real knowledge about that part. > Lucky enough it hit the repo during the merge window or I would have > reverted it in an instant. :-) -- Cedric BAIL ------------------------------------------------------------------------------ CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments & Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
