Frédéric, have you found out anything else regarding FishEye? I took a look, but the Flex project isn't there (in fact, there seems to be only one project, called "Default Project". It would be great to review each other's code using this.
Piotr, have you committed the TLF automatic test running code yet? It would be great to run the test automatically in flex too, not least because while trying to solve FLEX-34119 I had to create five separate unit tests. Alex, I think I've finally nailed FLEX-34119! Since I first wrote about it, it morphed into a total of five bugs: FLEX-34440, FLEX-34424, FLEX-34456, FLEX-34458, and the original FLEX-34119. All of them have unit tests, and now they all pass, which took a while. (Wow, I have to say, HierarchicalCollectionViewCursor was pretty broken.) There is a serious amount of code now on the FLEX-34119 branch[1]. It would really help if you (and potentially others) could take another look when you have time. Each commit has extensive information for what was wrong, and what the solution is. Let me know if you think any of those bugs could be solved in other ways. (Note that there are 7 or 8 commits which are not mine; they were introduced when I merged with develop.) [1] https://fisheye6.atlassian.com/graph/flex-sdk#branch=FLEX-34119&hl=Lineage&scrollToCsid=82d6b51694e846f0fb4e5505bf5a1b0601c21e7b On 22 July 2014 21:34, Alex Harui <aha...@adobe.com> wrote: > Regarding code review, we do have tools but for most changes, if the tests > pass you check it in and folks review the commit email. > Sent via the PANTECH Discover, an AT&T 4G LTE smartphone. > > Mihai Chira <mihai.ch...@gmail.com> wrote: > > > Hi Alex, > > > thanks for taking a look. I was also surprised to discover not only > that the parentTable variable wasn't necessary, but also that two > entire if branches (for add and remove) *always* ended up throwing the > 'Bookmark no longer valid' error (see the commit message here[1] for > details). In other words, they had never been tested. > > If there is some edge case, when we discover it we'll add a unit test > to make sure it doesn't return. > Speaking of which, I've just added the setItemAt operation to the unit > test, and I got some more 'Bookmark no longer valid' errors. I'll fix > these new occurrences hopefully tomorrow. Once I do and once I run the > mustella tests, I'll ask you to take another look before I merge with > develop. > > Two questions: > > 1. Would it be very complicated to run unit tests automatically in the > ant build script, or to the server CI process? > 2. Also, how do we do code reviews? Do we use some tool, like fisheye > or github, or just the IDE? I didn't find a wiki page about that. > > > Thanks, > Mihai > > > [1] > https://fisheye6.atlassian.com/changelog/flex-sdk?cs=4cb80bfbb37225b86b9e69cc4ec2fc21efe0a668 > > On 22 July 2014 17:49, Alex Harui <aha...@adobe.com> wrote: >> Hi Mihai, >> >> Thanks for taking on such a challenging goal. >> >> I don't see anything obviously wrong. IMO, the best thing to do would be >> to set up mustella and run the Advanced DataGrid tests. That's what our >> CI system will do when you finally check this into the develop branch. >> >> It is a bit puzzling that the old code messed around with the parent stack >> and it looks like you no longer need to. I wonder if there is some edge >> case associated with that. >> >> -Alex >> >> On 7/22/14 4:00 AM, "Mihai Chira" <mihai.ch...@gmail.com> wrote: >> >>>On the branch "FLEX-34119" I committed two unit tests and a fix for >>>this bug. If anyone would like to take a look, and run the tests, it >>>would be really helpful, because it's a trickier area of the SDK. >>>Could you let me know if: >>>* you think the unit tests are in the right folder >>>* the unit tests run as expected (see the README in >>>HierarchicalCollectionViewCursor_FLEX_34119_Test for some explanations >>>about it) >>>* it looks like the commits might break anything else >>>* you think the variable renames in HierarchicalCollectionViewCursor are >>>helpful >>>* we could run the unit tests automatically on each ant build. Is that >>>possible now? >>> >>>I'm still adding an operation to the unit test (setItemAt), and >>>looking to remove some duplicated code from >>>HierarchicalCollectionViewCursor.collectionChangeHandler. >>>I also plan to run the mustella tests soon, to make sure I haven't >>>broken anything. >>> >>> >>>Thank you! >>>Mihai >>
