For Crucible (code review), nothing we can do on our side, only INFRA could request it and I didn't ask them to do so (if you want to ask, it's up to you, it would be a very nice feature but you don't be in hurry), only FishEye has been setup while ago https://fisheye6.atlassian.com/browse/flex-sdk Frédéric THOMAS
> From: mihai.ch...@gmail.com > Date: Fri, 8 Aug 2014 17:21:04 +0100 > Subject: Re: [JIRA](ISSUE #FLEX-34119) Dev review request > CC: dev@flex.apache.org > > 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 > >>
