On 18/02/14 17:07, Michael Blumenkrantz wrote: > On Tue, 18 Feb 2014 16:15:02 +0000 > Tom Hacohen <tom.haco...@samsung.com> wrote: > >> On 17/02/14 18:03, Michael Blumenkrantz wrote: >>> On Mon, 17 Feb 2014 17:56:02 +0000 >>> Tom Hacohen <tom.haco...@samsung.com> wrote: >>> >>>> On 17/02/14 17:44, Michael Blumenkrantz wrote: >>>>> On Mon, 17 Feb 2014 17:31:24 +0000 >>>>> Tom Hacohen <tom.haco...@samsung.com> wrote: >>>>> >>>>>> On 17/02/14 17:09, Michael Blumenkrantz wrote: >>>>>>> On Mon, 17 Feb 2014 15:48:27 +0000 >>>>>>> Tom Hacohen <tom.haco...@samsung.com> wrote: >>>>>>> >>>>>>>> Hey guys, >>>>>>>> >>>>>>>> It's time to include Tiling2 into core. >>>>>>>> If you are not familiar with Tiling2, it's an improvement of the Tiling >>>>>>>> module I've been working on with help from billiob, cippp and >>>>>>>> discomfitor. >>>>>>>> >>>>>>>> Main features: >>>>>>>> * Very stable, especially compared to Tiling1. >>>>>>>> * Tiling is done in a tree, so you can essentially tile however >>>>>>>> you'd >>>>>>>> like. >>>>>>>> * Good support of floating windows. >>>>>>>> >>>>>>>> For more info: https://phab.enlightenment.org/w/emodules/tiling2/ >>>>>>>> >>>>>>>> I've been using it full time for a month now, and it seems to be stable >>>>>>>> (many thanks to discomfitor for fixing many related E bugs). It has >>>>>>>> been >>>>>>>> an external modules and has been tested by others, and there are no >>>>>>>> known issues. >>>>>>>> >>>>>>>> For all I can see, it's better than Tiling1 and introduces no >>>>>>>> regressions. I've talked to the Tiling1 maintainer (billiob), and he >>>>>>>> agrees that we should upgrade to Tiling2 in core, replacing Tiling1. >>>>>>>> Since it's based on the Tiling1 code-base (though a lot has been >>>>>>>> modified), and it serves the same purpose, I'll also rename it to >>>>>>>> Tiling, and not include it as Tiling2. >>>>>>>> >>>>>>>> For all I know, the only remaining concern about Tiling2 is something >>>>>>>> I'm not sure I'd like to fix (as I prefer the "broken" behaviour better >>>>>>>> than the solutions), and is not a regression, as it also occurs in >>>>>>>> Tiling1. It's an issue with not respecting windows' hints for min/max >>>>>>>> sizes, which are, as the name implies "hints". >>>>>>>> >>>>>>>> So, if there are no objections, I'll probably merge this into core >>>>>>>> Enlightenment 19 by the end of this week. >>>>>>>> >>>>>>>> Thanks for reading. >>>>>>>> >>>>>>>> -- >>>>>>>> Tom. >>>>>>>> >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> I appreciate the work that you've done to improve the tiling experience >>>>>>> under E and the time that you've spent to make these improvements. >>>>>>> >>>>>>> At present, however, I cannot yet support this merge due to the high >>>>>>> number of remaining bugs. In very early preliminary testing, I've >>>>>>> already found several serious issues. Let's revisit this topic for E19 >>>>>>> after a bit more testing and fixing. >>>>>> >>>>>> High number of remaining bugs? There was one "issue" that you reported, >>>>>> that I disagree with and already mentioned in my original post. Other >>>>>> than that, you haven't mentioned anything, phab is a proof of that. >>>>>> Everything works fine for me, and I haven't heard anyone else complain. >>>>>> You have the maintainer of tiling1 siding with this change, what are you >>>>>> talking about? >>>>>> >>>>>> Anyhow, while I appreciate your input for all things e, it's not your >>>>>> call to make. If you'd like to prevent this from going in on Friday, >>>>>> please provide some valid points. >>>>>> >>>>>> Also, if you have valid points to make, please make sure that they are >>>>>> relative to Tiling1, and not absolute. I.e if something is not a >>>>>> regression compared to Tiling1, it's not a blocker. Remember: this is >>>>>> not introducing a new module, this is updating/fixing/improving code, >>>>>> and replacing something that is broken for many people. >>>>>> >>>>>> -- >>>>>> Tom. >>>>>> >>>>>> -- >>>>>> Tom. >>>>>> >>>>> >>>>> >>>>> While I can appreciate your point of view regarding relative vs absolute >>>>> bugs, this is not how E development is going to operate going forward. >>>>> Getting something merged into E, regardless of whether it's a rewrite, >>>>> should not be a process of "it's good enough to get in, I'll fix the rest >>>>> of the things after I merge it". Merges should occur once there are no >>>>> remaining significant issues; a great example of this is the PackageKit >>>>> module from Davide Andreoli. After the merge, there have been almost no >>>>> commits to the module because it was already very well polished. >>>>> >>>>> Regardless of whether this is or is not "updating/fixing/improving code" >>>>> which already exists, it SHOULD fix the issues present in previous code; >>>>> otherwise, I don't see the point in merging it at all. >>>>> >>>>> You seem to be continually missing my key point: I am not opposed to the >>>>> merge in principle, I am simply opposed to it at this time. >>>>> >>>> >>>> Don't twist my words. I'm not saying "It's good enough to get in, I'll >>>> fix issues sometimes in the future". All I'm saying is, I have code that >>>> fixes many issues, and improves on many of the faults, it should go in. >>>> >>>> No it should not. When you commit code to E, do you fix all the bugs in >>>> e in one go? Of course not, it fixes some (many) of the issues, and >>>> improves in others while introducing no regressions. >>> >>> E is over 280,000 lines of code, excluding the theme (another 25000). Of >>> course I don't fix all the bugs in one go, that's clearly impossible for a >>> team of people, let alone a single person. Comparing something of this size >>> and variety to a tiny, specialized module with less than 3000 lines of code >>> is not sane or reasonable under any circumstances. >> >> What I'm essentially saying is: open source software is not created >> perfect and then released, it's created, and worked on, step by step, >> itch by itch. > > You seem to be saying that Tiling2, by being in a public E repository, is not > being developed using the OSS itch-scratching development method which you've > outlined, and that somehow merging it into core will noticeably change this. > I'm not sure where this claim/argument is going, but I disagree with it.
No, I do not seem to be saying that, it doesn't even make sense that I say this. Mike, stop trying to twist my words and rephrasing what I say in ways you see fit. Me saying what you implied I was saying doesn't mike the slightest sense in this context. What I'm saying is that E is an open source project. It's developed step by step by the community. We see flaws, we fix them. We see ways to improve, we improve them. I saw the tiling1 module was misbehaving, so I decided to work on it. I figured it'd be best for me to work in parallel (temporarily name it differently) to the original module, as to not interfere with my testers' (and my) work-flow too badly. I've managed to get it to a better state than what it was, that's that. > >> Tiling2 is at a state that's better than Tiling1 in every single way. >> It's an improvement over the current code base, that code base being e. >> My change to Tiling1 (i.e Tiling2) is like any change you make in e. >> A good comparison would be a component you've recently rewritten, twice, >> the compositor. It's severely misbehaving and has been for a while, and >> yet, it's in. That's fine, that's software development. You find a way >> to improve things, and you took whatever path you saw necessary. >> Tiling2 is a bit different than the compositor rewrites because the >> change is more surgical, and is an obvious stability improvement over >> the former version, not just usability and API (which is what the >> compositor changes bring to the table). > > Again you insist on comparing two very different and incomparable things; the > compositor rewrite was a core change of over 30,000 lines of code as well as > the core feature of E19; it was developed in a branch for 6+ months, tested > until the few users I was able to get for testing said that they had no major > bugs remaining and then merged. Any issues found after this point are > regrettable, but there was no other way to gain the exposure and testers > required for this; I know because I tried to get them. > > Tiling2, by contrast, is a rewrite of a tiny-but-invasive addon module. An > addon module which has a separate repository has no urgency or requirement > for merging, regardless of whether it is "better" than the existing module, > so I'm not sure why this thread is continuing even after I've made my > position clear. Furthermore, you've already explicitly stated that, in > addition to the nebulous claim that it is "better than Tiling1 in every > single way", it still isn't fixing some of the outstanding issues that > Tiling1 had (eg. T952 and others). I find these two statements hard to > reconcile and so I see no reason to expedite a merge of this unfinished > module. They are very much comparable and not too different. They are both improvements to core code. Sure, one is a loadable module, just like the evas rendering engines, but being a module doesn't make it any less core. It has been around since before our first release, and is part of our "promise" to our users. This is not a matter of including a new module into core (although even if it was, you'd still be wrong), it's a matter of patching existing source. Tiling2 is not a new module. I don't want to make this a discussion about the compositor rewrite, but as you said, tiling2 is tiny, compositor is huge and affects everything. Given the changes involved, testing tiling2 for 1 month is much more than testing the compositor for 6 months. It only has a separate repository because I wanted to get more testers, it was mostly developed in a branch on the main e repo. Also, those are irrelevant details. This thread is "continuing even after you've made your position clear", because while I respect your input, your position on things dictates nothing. It's not nebulous, it's a claim backed by all the users. We are writing software, and rely on user experience. All the feedback I got, including from the maintainer of Tiling1 backs this, and he (billiob) even replied on this thread. As I've said before, Tiling2 doesn't fix all of the issues Tiling1 had, just most of them. It's not an expedition. I've voluntarily slowed the process down because I respect proper testing and good development procedures. I developed my code externally. The normal process would have been to just commit to e directly. It's not unfinished any more than e is unfinished. > > The issues I found yesterday were the ones which I was able to find in the > first 5 minutes (literally) of testing, in between fixing some rendering > issues. The fact that I was so easily able to find so many easily > reproducible issues indicates to me that it is definitely not yet ready to > merge, which is consistent with my previous statement prior to having done > any testing. You found a few issues that were most likely caused by recent changes in E. E is very buggy, it's hard to develop in this environment where things change under your feet, get rewritten (recent sticky api changes) and etc. all of the time. Some things break when you break them, I know that, however most are being worked on. Some of the issues you found, were actually e issues, and everything you've found was corner cases that showed that you were stress testing non-normal cases (module loading/unloading in certain cases and etc.). Only one issue led to a segfault, and it was only after you unload the module (and was a minor oversight). Your statement is moot, if it was not, we could have just applied it on E and scrap the project altogether. Code has bugs, this code has less bugs than the one it's replacing. > >> >> I have reviewed all of the tickets you've opened on Phab, there were a >> bunch of them. A few were issues in E (not tiling), two of them I fixed, >> some were invalid, and some I couldn't reproduce. In any way, all of >> them were issues that exist in Tiling1, and thus are not regressions and >> therefore are in no way blockers for inclusion. >> >>> >>>> >>>> You seem to be continually missing my key point: you are wrong. I >>>> completely disagree with you on every point you've made regarding this >>>> issue. That's why it's up here for public discussion. >>> >>> If the public discussion finds that everyone is fine with ignoring my sole >>> point, which has been that I don't want to accept merges of code with lots >>> of known bugs, then I'm very disappointed in the community as a whole. >> >> To summarize what I've said above: >> 1. Compositor rewrite is known to have a lot of bugs and affects >> everyone, and yet it's in. >> 2. Tiling2 has no known *new* (compared to tiling1 that it's replacing) >> bugs. >> >> -- >> Tom. >> > > I stand by my statement that this is not getting merged until further testing > and fixing has occurred. Anything less would set a bad precedent for future > cases where people want to take external/rewritten modules and dump them into > core without adequate review/testing. > I understand that this may be frustrating, but you aren't the one who will > have to deal with such cases, so I'll ask that you cooperate here in the > spirit of open source and for the purpose of improving future development > practices. > I stand by my statement that this code is going in this Friday, unless there's any concrete objection going on. The rest of what you said about precedence, spirit of open source, frustration and cooperation is just fluff and your way of diverting the topic. I like E and the EFL, I don't want the to suck. That's why I'll do whatever I can in order to prevent you, or anyone else trying to make it worse than what it can or should be. -- Tom. ------------------------------------------------------------------------------ Managing the Performance of Cloud-Based Applications Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. Read the Whitepaper. http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
