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 >
