ToDo: Code Review Integration

CyanWorlds.com Engine Project Management
User avatar
JWPlatt
Member
Posts: 1137
Joined: Sun Dec 07, 2008 7:32 pm
Location: Everywhere, all at once

ToDo: Code Review Integration

Post by JWPlatt »

I believe in code review. Not only can it improve quality and security, but it can also educate others to new concepts or better ways of doing things. Plus, developers often enjoy knowing their work - their art - is appreciated by their peers. Lots of people read books. I've always thought it's a shame how much code is read only by its author. Open source is changing that, but I suspect not as much as it should be.

One of my line items is including Crucible in the JIRA workflow. As a start, I found this:
http://blogs.atlassian.com/devtools/201 ... klist.html

If you're interested, take a look. I haven't looked further yet, but I am concerned about this comment:
I'm still waiting for the ability to add crucible reviews into the workflow. Something along the lines as in open -> working -> reviewing -> closed. That way a review must take place prior to the issue being closed.
That's exactly what I was hoping to do on CWE, but the comment implies it's not there yet.
Perfect speed is being there.
User avatar
rarified
Member
Posts: 1061
Joined: Tue Dec 16, 2008 10:48 pm
Location: Colorado, US

Re: ToDo: Code Review Integration

Post by rarified »

We can define custom workflows; I just havn't tried yet. Another item in the workflow for some types of changes is a Uber-review by Cyan (or delegates). We should have a discussion on what constitutes a "fix", "enhancement", "changed api", and all other kinds of nonsense categories (not!).

_R
One of the OpenUru toolsmiths... a bookbinder.
User avatar
rarified
Member
Posts: 1061
Joined: Tue Dec 16, 2008 10:48 pm
Location: Colorado, US

ToDo: Code Review Integration (2)

Post by rarified »

I just initiated a review for cjkelly1's most recent changes to CWE-work. It appears to be waiting for the project "Moderator", which is currently set to Cyan. Don't know if Mark will be getting an email about it or not but I suppose for testing we shouldn't bug them yet ;) I'll take on moderator role in a little while and see what is getting sent. Have to get going on some other stuff right now.

_R
One of the OpenUru toolsmiths... a bookbinder.
User avatar
Mac_Fife
Member
Posts: 1239
Joined: Fri Dec 19, 2008 12:38 am
Location: Scotland
Contact:

Re: Client compile status

Post by Mac_Fife »

a'moaca' wrote:
rarified wrote:I just initiated a review for cjkelly1's most recent changes to CWE-work. It appears to be waiting for the project "Moderator", which is currently set to Cyan. Don't know if Mark will be getting an email about it or not but I suppose for testing we shouldn't bug them yet ;) I'll take on moderator role in a little while and see what is getting sent. Have to get going on some other stuff right now.
I can't find this review. I have poked all over JIRA (yes, not in a frameset) and I can't see any reviews listed.
I found it fairly easily in Fisheye (labelled as CWE-1) when I looked at recent activity, but when I tried to open it, my browser stuck at the translucent overlay window with the loading whirly thing in the middle. But my heavily firewalled office IE7 doesn't get on well with the Atlassian stuff and always seems to be very slow compared to my own PC at home (despite my office PC having twice the RAM and 4x the CPU horsepower and 100x the internet connection speed).
Mac_Fife
OpenUru.org wiki wrangler
a'moaca'
Member
Posts: 163
Joined: Sat Dec 13, 2008 11:22 pm

Client compile status (2)

Post by a'moaca' »

Mac_Fife wrote:I found it fairly easily in Fisheye (labelled as CWE-1) when I looked at recent activity, but when I tried to open it, my browser stuck at the translucent overlay window with the loading whirly thing in the middle. But my heavily firewalled office IE7 doesn't get on well with the Atlassian stuff and always seems to be very slow compared to my own PC at home (despite my office PC having twice the RAM and 4x the CPU horsepower and 100x the internet connection speed).
I did not know to look in Fisheye. I did not know how to get to Fisheye. From the wiki or forums this is completely non-obvious. I guess I am supposed to start from http://www.openuru.org?

Anyway, I found the review thanks to Mac. I had the same issue with the overlay. I pasted the same URL back into the window (pasting a URL into the window means "load this page" on my firefox build, and I love it). Oddly, this resolved the problem. I looked through the review.

I did struggle with the tool. The top lines of each diff window were cut off. I don't think I missed code, but there are links or something up there, covered up by the box with the revision thing and the View/Fisheye menus. I was not very confident of the horizontal scroll functionality, which is to say, for some files I felt unsure that what I was seeing wasn't cut off. When I clicked "Leave Unread" to see what would happen, it seemed to be permanent. I couldn't unclick it. And, the ones I didn't click that for don't list anywhere that I have reviewed them. It still says "A'moaca' Reviewer - 0% complete". Oh, well, trying again now, I can uncheck them, and it's marking the files read.

Oh, and I think it is slow too. Also my fan is running continuously, so I know it's keeping busy doing something. But those are probably not our problems to fix. I'm not much going to like using this, though. :(

I "completed" the review. I don't know if I just sent mail to Mark, heh. :) Did you get around to testing it out? I'm getting a little anxious about the reviews of my stuff, should I open them?

Also, this may have been a mistake, but I did not check in unmodified jpeg-8c so to review those changes we'll need to diff against jpeg-8c separately. Maybe I can upload a diff to Fisheye?

Edit: I can upload a diff. I didn't though, pending an answer on whether I should.

- a'moaca'
User avatar
Mac_Fife
Member
Posts: 1239
Joined: Fri Dec 19, 2008 12:38 am
Location: Scotland
Contact:

Re: Client compile status

Post by Mac_Fife »

I just completed the review that rarified kicked off. I spread it over two evenings as, like a'moaca', I found it uncomfortably slow. This is quite an old PC I'm using here and I found that IE was maxing out my CPU as I tried to navigate around the files or work scroll bars. I'm guessing it's due to the java running client side - I have problems with other java applications too, so it's not just Fisheye/Crucible.

I'd be happier doing the review "offline" and coming back in to post any comments and sign off the review. Except you can't see the "diffs" that way :?
Mac_Fife
OpenUru.org wiki wrangler
User avatar
JWPlatt
Member
Posts: 1137
Joined: Sun Dec 07, 2008 7:32 pm
Location: Everywhere, all at once

Re: Client compile status

Post by JWPlatt »

Look at the diffs through TortoiseHg. That's what I did. ;)
Perfect speed is being there.
User avatar
rarified
Member
Posts: 1061
Joined: Tue Dec 16, 2008 10:48 pm
Location: Colorado, US

Re: Client compile status

Post by rarified »

Mac_Fife wrote:I spread it over two evenings as, like a'moaca', I found it uncomfortably slow. This is quite an old PC I'm using here and I found that IE was maxing out my CPU as I tried to navigate around the files or work scroll bars. I'm guessing it's due to the java running client side - I have problems with other java applications too, so it's not just Fisheye/Crucible.
If you could try this again I'd be interested if there are any improvements with the 2.5.3 upgrade to FE/CU? Some of the bugs fixed in this release are looping javascript bugs.

_R
One of the OpenUru toolsmiths... a bookbinder.
User avatar
Mac_Fife
Member
Posts: 1239
Joined: Fri Dec 19, 2008 12:38 am
Location: Scotland
Contact:

Re: ToDo: Code Review Integration

Post by Mac_Fife »

rarified wrote:If you could try this again I'd be interested if there are any improvements with the 2.5.3 upgrade to FE/CU? Some of the bugs fixed in this release are looping javascript bugs.
Sorry, it's still like swimming in treacle :(
I can have several apps running including FE in a browser in the background, and my PC averages around 2% CPU utilization with cyclical blips to some higher level. As soon as I pass the pointer over the FE window (if it has a code diff displayed) the CPU maxes out. It is considerably more responsive in Safari than in IE though.
Mac_Fife
OpenUru.org wiki wrangler
User avatar
rarified
Member
Posts: 1061
Joined: Tue Dec 16, 2008 10:48 pm
Location: Colorado, US

Re: ToDo: Code Review Integration

Post by rarified »

Oh well; looks like they're assuming a more potent client. I've got a relatively new laptop with multicore Intel, FE seems quite responsive in Opera to me. We have what we have ;)

_R
One of the OpenUru toolsmiths... a bookbinder.
Locked

Return to “Management”