It seems it has changed [1], let me cjeck again with them. Frédéric THOMAS
[1] https://www.atlassian.com/software/crucible/pricing > From: webdoubl...@hotmail.com > To: dev@flex.apache.org > Subject: RE: [JIRA](ISSUE #FLEX-34119) Dev review request > Date: Tue, 22 Jul 2014 19:36:50 +0100 > > Hi Mihai, > > > 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. > > Unfortunately, even though we have Fishyey and Criduble installed, the later > is not part of the open source offer, that was what I was told when I > requested it. > > Frédéric THOMAS > > > From: mihai.ch...@gmail.com > > Date: Tue, 22 Jul 2014 18:18:50 +0100 > > Subject: Re: [JIRA](ISSUE #FLEX-34119) Dev review request > > To: dev@flex.apache.org > > > > 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 > > > >
