Author Archives: michechuang

Meeting Minutes for December 2, 2012

Announcments:

  • yangtina was absent from the meeting due to an exam, but will be present at the post-mortem.
  • This is the last official meeting of the term.
  • Post-mortem will be held on the Sunday after next (December 16).
  • This Tuesday (December 4th) is firm pencils down at 11:59PM PST.

Questions / Answers / Tips:

Q (slchen): Regarding your comment here: http://reviews.reviewboard.org/r/3567/ regarding possibly adding XML thumbnails as part of the XML Review UI Extension – I’ll have to do some digging and add a new hook in reviewbuard/extensions/hooks.py, but I think it’s doable. How should I proritize that?

A (purple_cow): Low.

A (slchen): Here are the possible tasks on my plate: first and foremost is getting the commenting functionality done for TextBased Review UIs. I have been working on that with aam1r since yesterday, and he’s doing most of the backbracing on it since I’m much less familiar with JS. I think we have most of it done, but aam1r’s mostly working solo on it now and I’m just helping him trouble shoot when he runs into a block

Q (m_conley): does it look like something you’ll be able to drop-in and use for XML?

A (slchen): it is, we are trying to generalize it for Text-Based Review UIs in general

Q (slchen): and the other thing I’m looking into is documenting other hooks; so my other question is are there anything else for me to do in case I get blocked on any of the current tasks I have?

A (m_conley): considering the closeness of pencils down, you might already have enough

Q (tahnok): so buildbot can apply patches to a branch/revision other than master and reviewboard has a branch field, should I be getting the branch field in the reviewrequest? or does reviewboard store branch information internally?

A (smacleod): As ChipX86 said, that branch field really is just freeform text

A (ChipX86): you can’t trust it

A (purple_cow): we should maybe look into adding a hidden field for the base revision of a change, which post-review could fill out

Q (tahnok): but patches can applied to anywhere. should I add an option to use that field?

A (ChipX86): we have a base revision, the source revision of a file

A (purple_cow): but that’s per-file, and to build, we need the whole tree

Q (ChipX86): can we not figure out the commit from that?

A (purple_cow): not that I know of

A (tahnok): at least that way users would know that if they need to use a different branch they can, and there’s no reason why the patch has to be applied against the base-rev. it might make more sense to have it applied to HEAD. I think that by default it uses master, but there’s an option to use the branch field

A (purple_cow): let’s start with master for now

A (ChipX86): what would be kind of cool is to say “Here’s a list of branches, make sure it works on all”

Q (tahnok): could the list of branches be global for the plugin? there could be an option field that takes a comma seperated list of branches?

A (smacleod): Yeah, I think that works

A (tahnok): I’m just worried that you could have reviews for different projects on the same installation, and that they wouldn’t all have same branches.

Q (smacleod): hmm, sorry to throw another wrench in this, but Review Bot isn’t repo intelligent at the moment. Doesn’t discriminate between repos šŸ˜›

Q (ChipX86): we could attempt to look up by the field and hope. then if an install uses buildbot, they have incentive to make sure it’s correct. if it’s blank, assume master. it’s just that it is freeform. when DVCS stuff starts to happen, there will be a better concept of branches

A (aam1r): no questions. as slchen mentioned, we have been making good progress on the comments. we got it working for markdown. we are now generalizing it so it works for any text-based review UI. should be up for a review request today.

Q (Allyshia): So I have updates to my latest review comments in the works — just looking for some indication (based on conversations with some of the mentors, status reports) of what seems to be incomplete that should really go in before Tuesday (by the end of my current changes, JSLint should be “complete”, manual trigger will still be in progress, but my portion should be complete … unless there was anything else that someone thought of that should get in right now).

Q (m_conley): what’s left to do with manual trigger?

A (Allyshia): well the UI will be done, but the actual trigger functionality won’t be complete, neither will permissions

Q (m_conley): the licensing thing got all sorted out?

A (Allyshia): with JSLint, yes.

Q (Allyshia):Ā I’m having this frustrating problem regarding updates to ReviewBot (I get the feeling my local repo is out of sync but won’t sync) but I think I can troubleshoot that with smacleod afterwards if he’s OK with it

A (smacleod): yeah, we’ll fix it for you

Q (johns187): I’m going to refactor my code, write ‘rb publish’ — what other commands can I do?

A (purple_cow): at this point I’d really like to see some effort on removing the unnecessary options instead of other commands

A (jzamb): I don’t have any questions at the moment. Just want to say i just posted for review and it’d be nice to get some feedback.

A (Karl-L): No questions, but still need more review for r/3488. It’s been through a few rounds, and the issues raised up until now have been dealt with.

Q (Michee): I still haven’t figure out how to view localhost outside of virtualbox, I don’t know if anyone can help me. So as of this point, I’ve only tested my image diff page on Firefox in my (extremely slow) ubuntu inside virtualbox.

A (m_conley): I might be able to, after the meeting. I think for VirtualBox, there’s a portmapping thing you’ve gotta do.

Q (Michee): That’s also something I’m a little concerned about. My OS runs really slowly so I don’t know if that might make me see things differently.

A (m_conley): the rendering should give the same result, regardless of the speed. the interaction might just be sticky. :/

A (Michee): I just pulled my code out of WIP so a code review would be muchly appreciated.

Other:

m_conley: I just wanted to say that this has been a fantastic term. I’m really pleased to see how we’re getting students reviewing other students, etc. big thumbs up for the folks who’ve been doing that. I also wanted to remind you all that no matter what, when this course is over and done with, you’re still welcome to hack with us on Review Board. we’re going to have a new crop of students next semester, and they’re going to need help and reviewers. and there’s always plenty of work. and we like you folks. šŸ™‚

ChipX86: (the work! It won’t end!)

m_conley: So please, stick around if you’re having fun. Yes, we’ll always have work to dish out. šŸ˜€ Great work everybody.

ChipX86: if you liked what you did, if you want to in a sense “own” some of that work going forward, we’d be very happy to have oyu on board

m_conley: the more the merrier, really. it always makes me happy to see UCOSP names in the release notes

ChipX86: and if any of you are ever down in the Bay Area/Silicon Valley, hit us up šŸ™‚

m_conley: ditto wrt Toronto. I lunch.

smacleod: (I’m in cincinnati, so if for some crazy reason you end up in OH, let me know… probably not likely tho haha)

ChipX86: and feel free to remain here in IRC. anyone who’s planning to use [the bouncers] to be at least somewhat active in here, stay signed on and I’ll keep them going. otherwise, quit so I can see later what to prune.

(tl;dr:Ā Continue to hack on Review Board if anyone is interested. Hit up ChipX86 in the Bay Area/Silicon Valley, m_conley in Toronto, smacleod in Cincinnati. Stay on IRC if you want to keep your bouncer.)

Advertisements

UCOSP Blog Post – October 30, 2012

I’m Michelle Chuang, a 4th year Computer Science and Biology student at the University of British Columbia. Like most of the people here, I joined the Review Board team through the UCOSP program after hearing several of my friends speak highly about the program. When I discovered that Review Board was one of the projects available, I became so excited. I just completed a summer internship at Amazon where I had to use Review Board extensively for our code reviews. Getting a chance to contribute to a tool that I had used all summer and impacts so many people was something I couldn’t pass up. In addition, I thought it would be a great chance to strengthen my Python coding skills, learn about the Django framework and gain experience in front-end web development.

Appetizer

At our code sprint in Waterloo, I started by tackling a few “Easy Fix” bugs. The first issue was a report that URLs to invalid files in review request diffs returned a 404 page with an empty respond body instead of a 404 page with an error payload. It was an interesting way to start my exploration through the Review Board code base, as the issue turned out not to be a bug but a result of browsers and certain plugins hiding the payload. However, it gave me an opportunity to dive deep into the areas of the code that handled URL requests. My second issue was a missing 404 for URLs to diff comments from an invalid diff or file. Luckily, this turned out to be a real bug that I could acutally fix and I eventually traced the problem back to an assumption that if the review request was valid, so were the diff and the file. A few new lines of code here and there and I thought I was ready to ship my code. Unfortunately, my bug fix caused a unit text to fail and it took a few hours of further examination to realize that I had discovered a new bug that had been previously hidden by the bug I had fixed. Despite having to fix this new bug, the fix allowed me to learn how the unit tests worked and gave me an immense appreciation for the sheer size and complexity of the Review Board code base.

Main Entree

My main project is with Pluggable UIs, which is an extremely useful addition to Review Board’s functionality. Originally, it was decided that I would work on PDF reviewing UI but was redirected towards image reviewing UI after some workload analysis. Consequently, I haven’t been able to make as much progress in my project as I would have liked, but looked into several resources for image reviewing. My model for image reviews is Github, which utilizes four different ways to view and compare images: 2-Up, Swipe, Onion Skin and Difference. Ā The goal is to have each of these views (or similar views) implemented by the end of the semester

2-Up involves viewing both images side-by-side without any addition bells and whistles, and is particularly useful if the image has changed size. Swipe overlays both images atop one another and allows the user to view portions of the original and new image depending on where the swipe slider is. Onion Skin also overlays both images, but allows the user to change the opacity of the images to fade in and out of original and new images. Lastly, Difference highlights the pixels that are different between the two images by calculating the difference of each pixel per RGB channel.

One thing that still needs to be decided is whether the image processing will happen client-side or server-side. Ā Python has numerous libraries that support image processing, which will need to be done in order to compare the images, and the one that Review Board uses is Python Imaging Library (PIL) so this seems like an ideal option. However, the Swipe and Onion views will likely be implemented solely in Javascript as it does not require the same degree of image processing.

There are several other ways to represent the changes between images. Ā One such way is using the percentage difference between images. Ā The percentage difference between images (of the same size) can be computed by summing the difference of each pixel per RGB channel (i.e. three differences per pixel), and divided by the total number of components (number of pixels times three), divided by 255 for the number of possible values for each RGB channel. This is also similar to how Github generates their Difference view via RGB channel differences. Alternatively, pixels can just be compared as a whole instead of through each channel individually to generate the percentage of pixels changed and which pixels are changed. With this information, we could display the changes with a bounding box around the area that contains the changes. A problem with this approach though is that if the image is only modified slightly (such as darkened), the change is considered equivalent to if the image was changed completely despite being similar from a human eye’s perspective. Lastly, changes can also be represented using theĀ Ī”E*Ā distance metric to calculate the color difference. This approach is comparable to Difference view approach that Github uses and can be used to generate a difference view using shades of grey.

Dessert

At the end of the semester, UBC requires its UCOSP students to give an oral presentation of their work to the university’s supervisor of UCOSP, where we discuss the project in general, our specific contributions to the project, and what we feel that we learned from the experience. Ā It will be interesting reaching the end of this semester and reflecting on the progress that I’ve made on Review Board. Ā I feel like UCOSP is such a great program for students, even those who have done internships or co-op, since it allows us to continue to work on projects that have a real impact rather than just one-off assignments here and there. I hope to make the most out of this experience, and would love to continue contributing to Review Board after this semester.

Status Reports — October 7th, 2012

Michelle Chuang

Currently
  • Trying to figure out why my unit tests fail after fixing the bug for issue 2552
  • Reading through the code reviews that Christian has submitted to get a better idea for Pluggable UIs
Roadblocks
  • I may need more help with trying to figure out why the unit test is failing (I believe Mike was helping me with it during the code sprint).
Next
  • Talking with Aamir to decide which formats for Pluggable UIs we’re going to tackle first, and how we should go about doing it.
Questions
  • None.

Allyshia Sewdat

Currently
  • Moved from bug fixes over to project work : implementing the first (of possibly several) new static analysis for Review Bot. Done some research, got some feedback from the team, going ahead with the implementation.
Roadblocks
  • None, but will keep closely in touch with Steven and the rest of the team if any questions arise.
Next
  • Finish implementation of JSLint tool and submit a preliminary review to the ReviewBot review group for feedback asap.
Questions
  • None.

Aamir Mansoor

Currently
  • Reading through Chris’ pluggable UI review requests to understand pluggable UI framework
  • Trying to manually merge in Chris’ pluggable UI requests that haven’t been shipped yet
Roadblocks
  • I am having a hard time manually applying the diff patch for request #3342. It seems like the patch has some conflicts and needs to be updated.
Next
  • Talk to Chris and Michelle to figure out what the next steps are and what formats we will initially begin with
Questions
  • None

John Sintal

Currently
  • Stepping through (with Pdb) postreview.py and trying to understand itsĀ multipleĀ usages.
  • Ran through the tutorial Steven gave me, successfully posted a review with the RBClient with all the extra (txt file attachment, etc)
Roadblocks
  • I am not sure what to do. Read more documentation or attempt to implement a command.
  • Steven’s base class that I am supposed to use aren’t complete (last time I checked, which was at the sprint). I’ll have to talk to Steven about this.
Next
  • Talk to Steven about the base class and see what’s an entry level script I can start coding. By that time, I should familiar with postreview.py.
Questions
  • Steven, you said I shouldĀ consultĀ postreview.py. Once I am done that, what is my next step? Should I look somewhere else to grasp a betterĀ understandingĀ before I start to “write the scripts”?

Karl. L

Currently
  • Looking at GitHub API and current RB code that uses it.
Roadblocks
  • None.
Next
  • Add SSH key association to group of already implemented GitHub API helper functions. Plan out how auto key association will work with existing hosting service configuration forms.
Questions
  • Nothing major. Will save them for Sunday.

Sampson Chen

Currently
  • Looked for a way to automate status report collection; doesn’t seem feasible. (Google Forms + a formatting parse script still involves manual work. Building an app just for this is too much overhead and time better spent elsewhere.)
  • Moving from looking at bugs to reading through Christian’s resources for pluggable UI + Mike’s hint for existing thumbnail infrastructure.
Roadblocks
  • Not a block yet; just committing time to get a good mental picture of how to implement thumbnails,
Next
  • Continue looking at the code base / architecture and collect a list of questions to ask in the meeting tomorrow.
Questions
  • Speaking of Canadian Thanksgiving, what do y’all talk about over thanksgiving dinners with your families?

Jesus Zambrano

Currently
  • Working on jQuery tutorials
  • Setting up dev environment on a new machine
Roadblocks
  • none at the moment
Next
  • Make changes to review_header.html as mike suggested and post it again for review
Questions
  • None right now

Tina Yang

Currently
  • Putting print and console.log everywhere to step through the process of how an file / screenshot is uploaded and learning javascript/python along the way
Roadblocks
  • Trying to recover from mind being blown by review.js, datastore.js and postreview.py
Next
  • Step through and learn about how postreview grabs the diff from different VC (basically rbtools/postreview.py and rbtool/clients/*)
  • Trying to figure out why error messages attached to “Add Screenshot” popup won’t get cleared if new errors come in
Questions
  • I’m a visual learner as well and Steven’s diagrams really helped me a lot in understanding the awesome but complex structure of rbtools. I Ā am attempting to use some tools to aid me in grasping the architecture of the reviewboard package as well (eg.Ā https://github.com/dzhibas/django-yuml) Is there a visualization of the architecture of reviewboard somewhere already? I feel like this could be really useful to students since a lot of the structures I see in reviewboard remind me of the good design pattern/practices I’ve learn in software architecture class. (side note: I think reviewboard should be in the 3rd volume of thisĀ http://www.aosabook.org/en/index.html)
  • For bugs, does confirmed mean that one of the developers was able to reproduce the bug or does it just me accepted as a defect/enhancement?