Author Archives: jzamb

Status Reports – Nov 23rd, 2012

Jesus Zambrano
– Bind “.editable” elements to the inlineEditor() when a user re-opens a review request that was previously discarded.
– Adding a field to RB.ReviewRequest to be able to check whether a review request is public or a draft.
– I’m not really sure how to go about doing point #2 above. So far(As told by ChipX86), I’ve created a function _loadDataFromResponse() in RB.ReviewRequest that gets called in _checkForUpdates(). But i’m not sure how that helps me when trying to check whether a review request is public or a draft.
– Solve Roadblocks
– None
Allyshia Sewdat
– Updated JSLint patch as a major review was done (Thank you Mike), made some changes to functionality — awaiting feedback on my counter-comments.
– Did some reading about extensions + hooks in reviewboard
– Discussed design with smacleod regarding manual trigger
– Add that UI and hook up the manual trigger ASAP, considering future permissions concerns
– Would be appreciated if I could get some final comments on the questions I asked in the  JSLint review #3435. Thanks!
Aamir Mansoor
– Finished addressing issue for “Markdown Pluggable UI“. Would love any more feedback on it
– Addressed most of the issues for “WIP – Rendering pluggable UIs in lightbox“. Still working my way through it.
– Had issues with git because of the way I was merging/rebasing things (Thanks Christian and Mike for helping me fix those!)
– Continue working on rendering review UIs in lightbox
– Hopefully get a “Ship It” on Markdown Pluggable UI over the next few days
John Sintal
– Finished setting up svn on local environment and dev server, able to post reviews with svn
– Working on rb patch
– pX arg for svn — a really morale killer here, been struggling a lot. Definitely impeding me.
– patch just isn’t working (see question below)
– get pX logic working with svn
– talk to steven about the options that aren’t needed
(Please excuse this huge question)
Patching works fine if I create the diff manually.
i.e make some changes, ‘svn diff > diff.txt’, revert changes, then run ‘patch < diff.txt’
However, this way fails consistently:
1. svn checkout in some folder, called /repo1
2. Go to trunk/src/opl/textc and edit, and save that file
3. post-review to dev server and publish. Verify the diff is correct (changes to
4. svn revert -R . (put things to before state so as to apply a patch).
5. rb patch <rid>
6. We get a command like this [‘patch’, ‘-p0’, ‘<‘, ‘/tmp/tmpzDcnC9’]
— doesn’t work, i’ve tried with -pX, -p0 to -p5, all fail, what am I doing wrong? —
Error Msg:
patching file
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED — saving rejects to file
Here are some state variables
‘base_path’: ‘/trunk/src/opl/textc’
contents of downloaded diff:
Index: /trunk/src/opl/textc/
— /trunk/src/opl/textc/ (revision 131)
+++ /trunk/src/opl/textc/ (working copy)
@@ -1,4 +1,4 @@
-package opl.textc;
+package opl.textcooo;
 import java.util.ArrayList;
Command attempts (w/o Python)
patch < /tmp/tmpfB7d8S
patch -p0 < /tmp/tmpfB7d8S
patch -p5 < /tmp/tmpfB7d8S 
=(all fail, giving a msg like so)==> 
patching file
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED — saving rejects to file
Tina Yang
– Fixing the issue where the table cells are not aligned properly in Chrome
– Refining the UI for inline file attachment
– None
– Found out that sidebyside text diff cells are not the same size, trying to resolve that by making them the same size and aligned with the inline file attachment sidebyside view.
( Issue explained: : notice how the lhs of the text diff table cell is a bit wider. For this image in my Chrome browser: both the cells for the inline file attachment are 782px wide; but the lhs of the text diff (col #2) is 777px wide but the rhs (col #4 – AKA the new file side) is only 747px wide. Unless this is by design, I will attempt to fix this. )
– We can’t delete a file attachment from a review request yet, is that correct? If so, is there any tips on clearing up the review request? I have a lot of screenshot files on my review requests that are no longer applicable, I think they make the review request somewhat confusing and want to clean them up somehow. Would labeling them by revision number in the caption help? Any tip on good practice in this case?
– Addressing issues raised in Buildbot plugin review
– Trying to figure out a sane way to supply per-worker configuration
– None
– Getting SSH buildbot integration working
– Get review branch and pass it to buildbot
– Make the output prettier

– Is there any kind of comment formatting that I can use to colour the output of the build?

Sampson Chen
– Implemented mem caching for thumbnail project on .rst and .md attachments
– Refactored the Text-ish classes in to remove duplicate logic; ReStructuredTextMimetype and MarkDownMimetype now subclass TextMimetype instead.
– Implemented infrastructure for registering MimetypeHandlers in much the same manner as registering Review UIs, replacing the old way of doing it through __subclass__
– Implemented protection against js injection attacks while using docutils / markdown modules
– None atm, just putting a lot of time into to resolving all the loose ends for thumbnails project to get it ready for patching, so I can get back to the extensions project.
– Test some mockups with the extension project
– None atm
Karl Leuschen
– Cleaned up remaining issues and landed patch dealing with checking if SSH keys already associated.
– Incorporated patch into WIP review request
– Modified WIP request (r/3488) based on feedback from Christian
– WIP request is functional, but still needs review before it can be moved out of WIP
– All clear.
– Fix issues with r/3488
– Reviews
– None for now
Michelle Chuang
– Working on getting images overlayed for a Swipe and Onion image diff view
– Adding functionality for Swipe view
– All clear.
– Posting a code review ASAP
– None for now

Meeting minutes for Nov 18th, 2012


  • Reminder pencil down dates is December 4th, 2012. So blog posts and patches posted after this time will not be considered during your evaluation.
  • Note that it’s possible that your code might be in a fine state to land, but we don’t land it. So, target getting stuff up on RB, and getting “Ship it’s” – not necessarily landing.
  • John was absent for this meeting. Mentors were noticed in advance.
  • If your school has additional requirements for you (papers, presentations, etc), make sure we’re aware of that. (Allyshia,Tahnok, michee, yangtina and jzamb have extra requirements).


Q:(yantina) Will it be helpful for the final review if we round up our work for the term and send an email to all the mentors with links to what we did? (reviewrequests(we posted or commented on), blog posts, link to presentation slide for this project etc)

A:  We’ll be able to view your blog posts and review requests without difficulty. Your presentation / outer course-work isn’t something that we should be factoring into our evaluation. So you can include it for interests sake, but we won’t be factoring it in. Might be good blog fodder though and blog posts we *do* notice.

Q:(tahnok) I can’t seem to get buildbot to accept. It barfs when I try to submit a patch., mainly fatal: git apply: bad git-diff – inconsistent new filename on line 15.

Q(m_conley): And you can apply the diff without difficulty locally?

A(tahnok): Not sure, i didn’t try.

A(m_conley): ok, give that a shot – let’s see if we can remove BuildBot from the equation

Q(smacleod): Also, what command did you generate that patch using?

A(tahnok): git format-patch master –stdout > fix_empty_poster.patch. the line in question being — /dev/null. +++ b/bot/reviewbot/tools/

A(purple_cow): it looks like there’s actually several patches in that “patch”. probably because you had several commits

A(tahnok). Yeah, there was a bunch of commits. But I tried with single commits as well

A(m_conley): Alright, can you stick around after the meeting?

Q(slchen): How difficult is it to work with memcaching, anyone with more experience than I? just wondering to help prioritize tasks,

A: memcache is a pretty simple thing to use. what are you wanting to use it for?

A(slchen): ChipX86 mentioned that for the thumbnails projectm_conley morning I changed the parsing alg to read up to the first 2000 chars of a file.

A(purple_cow): The easiest way to use it from our code is the cache_memoize method

Q(slchen): Would you say it’s something I can learn and apply within an hour or so?

A(purple_cow): sure.

A(m_conley): cache_memoize is defined in djblets.util.misc. Some documentation in there you should read

A(purple_cow): basically you create a cache key that’s unique to your particular data. And then call cache_memoize with that key and the function that computes the data

Q(slchen): Does it update automatically?

A(purple_cow):If it’s in the cache, it will use that, otherwise it calls the function and stores it for the next time

Q(michee): Should I be concerned about making the image diffing too similar to what github does?

A(m_conley):I don’t think that should be a concern, no. In fact, there might be a lot to learn about their design. Prior art is sometimes a great place to start from and improve upon. And users get to inherit the familiarity, which is a bonus,

Q(michee): For the hard deadline, should we be posting code reviews by then or getting ship its by then?

A(m_conley): You should be getting to the ship-it state for the hard deadline.Though, a reminder – you are absolutely *absolutely* welcome to keep hacking on your project after the deadline passes.

A(purple_cow): we’ll still take into account anything that’s posted, but putting up new stuff right before the deadline won’t get as much of a look from us because we’ll be busy writing the evals.

Q(Karl-L): I want to issue a gentle reminder that I’m still in need of some pointers on r/3488. ChipX86 has been doing most of the reviewing for my project specific code, so I’m fine to wait for him to return from vacation if he’s the best person to look at it.

A(mike_conley): I’ll take a peek after the meeting, after we tackle tahnok’s problem

Q(Allyshia): As per the questions that ChipX86 raised regarding the reviewbot manual trigger mockups, can I have a quick confirmation of why we chose to go with the implementation of manual trigger in the first place, to be sure? What are the needs we are addressing, what use cases are we covering.

A(smacleod): We have automatic, and there could be cases of tools we don’t want to automatically trigger, but would still like the option of using. I’ll use as an example: Anyone can post code, so if we automatically trigger say, a builbot plugin, for every review. It could allow people to just run arbitrary code through our buildbot. Thus, manual triggering with permissions seems like a good option to have

Q(Allyshia): Conceptually speaking the manual trigger in itself is not an extension, correct?

A(smacleod): Reminds me of a gotcha -> Not sure how extensions can create and manage permissions. The manual trigger is part of the Review Bot extension not its own. I think we need a more lengthy discussion on the topic, so yeah, lets postpone that to outside the meeting

Q(Amiir): Just want to say that I took r/3434 out of WIP. would be great if someone could look at it so I can fix any issues or get a Ship-It

A(m_conley): Awesome,can do.

Q(m_conley): How is the Lightbox stuff going?

A(Amiir): it’s going well. I pushed a WIP for review yesterday. It’s currently using modalBox

A(m_conley): excellent. Sounds like a good place to start

UCOSP Blog Post

Part 1- Intro and expectations for the semester:

Hi there, I am Jesus Zambrano and I’m a 4th year Software Engineering student at university Of Ottawa. I first heard about UCOSP through e-mail. Once I started reading about it I got really excited and decided to join UCOSP. I really liked the idea of working in a distributed team with students from other schools. In case you didn’t know, UCOSP organizes a code sprint per semester. At the sprint, students and their mentors gather and work on their assigned projects. This year’s sprint took place in Waterloo.

The project that I’m going to be working on is Review Board, a web-based tool that eases the code-review process for developers. More specifically, my task within Review Board will be to reduce the number of full-page reloads to move towards dynamic updating of the page content. I’m new to some of the technologies used at Review Board, but I’m hoping my effort will make up for it.

I expect this experience to be challenging but very rewarding in the end. I think it is a great way to really prove your self and show what you are capable of as a starting developer. This will be my first time working on a project where my input will potentially have an impact on real customers. It will be very fulfilling to see that my work has helped someone in some way. Which is really the essence of this profession, solving people’s problems.

Part 2-Thoughts on the sprint:

The code sprint at Waterloo was a great experience.  The mentors are extremely helpful and know how to walk students through their problems. They do a really good job at motivating you to work hard (you don’t get your ship-it shirt until you fix your first bug!). I also enjoyed the environment we worked in; everyone was laid back and had a good sense of humor. But at the same time we were very serious about our work and getting things done. With Review Board, you really get a sense that we are all a group that is there to learn from each other as much as possible. For anyone reading this that is considering joining UCSOP, do it! You will have a great time. Thank you Steven, Mike, David and Christian for helping us and for your commitment to UCOSP!