Author Archives: jsintal

About jsintal

I am student at University of Toronto studying computer science.

Status Report — November 10th, 2012

John Sintal
– Still working on rb patch. Almost done, it’s just that last 5% that’s in my way.
– Figuring out how to properly testing rb patch and rb diff.
– As said here, I am having issues with executing the command in Python. The command works fine otherwise for most cases.
– Proper pX argument. I believe ChipX86 said he had a friend who had the logic on figuring out the best configuration.
– Testing could be challenging and impede my work on new commands. It’s been discussed by Steven that testing isn’t trivial for this sort of stuff, especially with all the SCMs and a lack of a testing structure for rb commands. I forsee myself spending a lot of time on this as opposed to working on new commands.
– I am finding code review to be tough since most the review requests are already tackled by the mentors. I also wish there were more opportunities to look for pep8 style violations, as those are easier to detect. Perhaps it would be possible to let students tackle those types of problems (mentors purposely leave them). Altho, my idea might just be crazy and I just need to try harder in terms of picking new reviews.
– Finish rb patch which includes the next two tasks:
    – Code clean up (decide with Steven on refactoring decisions, which options to leave in/out, assess the options that are superfluous yet still are required by legacy code)
    – Testing on many SCMs
    – Getting the cmd to actually execute in the Python script
    – Figure out the pX logic 
– Find review requests to review.
– Is there an option to receive notification of anyone posting a review? This way I can be first to look at the review.
Aamir Mansoor
– Finished addressing issues in r/3434
– Not sure what to work on next for Markdown UI. Some ideas come to mind: work on the frontend for what I have done so far or move on to another file extension while my latest changes get reviewed.
– Figure out what to work on next for Pluggable UI
Tina Yang
– Modify diff_file_fragment.html to display the binary files inline better
– Add unit tests and possibly more manual test cases
Allyshia Sewdat
– Getting the non-WIP version of the JSLint plugin review request ready
– Putting together mockups for the static analysis manual trigger UI
– Was held up by trying to figure out the best way to get JSLint to execute with custom options. Behaviour was unexpected whereby passing in select custom options would have JSLint stop sooner (scan less of the file) than passing in either no options or ALL options including those explicitly set to the same value as default. Experimented with online version and it does not seem to behave the same way. It would be a poor user experience to scan 18% of the file if they are expecting it to scan 75%. Trying to figure out why this is happening, but fallback solution that has been tested is to pass in explicit default values for all parameters which RB users do not configure via admin panel.
– Focus on second project : manual trigger.
Sampson Chen
– Reading about how memcaching works in RB
– Started coding for Extension integration with ReviewUIs
– Been very busy this past week, but I should get uninterrupted time to work on Sunday.
– Feels like I’m progressing very slowly with the extensions project. I’ve read over the code in reviewboard/extensions, djblets/extensions, reviewboard/review/ui, made notes as I went along, referred back to IRC conversation history with mentors multiple times – but it’s still fuzzy in my head about exactly how to proceed.
– Get a WIP review-request for extensions project up ASAP.
– Add more polish to the thumbnails project (style issues)
– Possibly add memcaching to thumbnails project
– None right now.
Karl Leuschen
– Put up a new WIP request for associating SSH keys from form (r/3488)
– Added review request for checking the existence of an SSH key among repository deploy keys (r/3478/)
– No roadblocks, but did make the mistake of spending too long waffling on some issues before posting a WIP request. Will try not to do that again.
– Get started on working with BitBucket API and look at some of the other supported hosting services.
– Keep working on active WIP request.
– Have a look at some of the other active review request.
– No general questions at the moment, but any feedback on r/3488/ would be appreciated.
Wesley Ellis
– Configuring buildbot to accept try scheduler
– None at the moment
– Add options to buildbot plugin
– None at the moment
Jesus Zambrano.
– A user should not be able to edit the request’s fields once the request it’s discarded. Currently working on doing that without reloading the page.
– Trying to figure out how to change the status of a review request when it’s discarded.
– Can’t figure out how to reset the value of time.timesince when the request is discarded.
– Allowing a user to edit the fields again if the request is reopened
– None.
Michelle Chuang
– Modifying ImageReviewUI and Javascript to display two different images side by side

– None at the moment

– Complete the two-up view and post a code review for it
– Start working on an onion skin view

– None at the moment

[WIP] Working with rbtools and its commands

Jzamb said he was going to start work with rbtool’s commands, so I thought I’d write a blog entry that helps new ppl get acquainted with the work flow. Please note that I will update this as I get a better understanding of the rbtool commands codebase [WIP]. Excuse spelling, format, grammar errors. Feel free to email/comment for any misunderstandings or corrections.

Highlevel Concept – The idea behind “RB Commands” is to make a uniform way of interacting with Review Board’s API. The user would type something like “rb <cmd> <args>.” As you can see, the prefix “rb” is used for all commands. This is similar to “post-review” and in fact, most the things that rb commands will do are already performed by the post-review command. However, this new approach to commands is cleaner in the sense that 1 script = 1 command, versus 1 script = many commands.


Currently, much of the rb commands are in an “experimental” phase where the code is held on Steven Macleod’s repository. Not sure when it’ll be migrated to the main codebase/master, but for now it’s here.
If you want to pull the code down locally, do the following:
1. Change working directory to the rbtools folder
2. Add my repository as a remote:
    git remote add smacleod git://
3. Create a tracking branch for the rb-commands branch:
    git checkout -t smacleod/rb-commands
Depending on your version of Git, the last command may not work. Please let me know, as I’ve lost the exact command I used to pull down the branch.

Code tour 

Adding a command involves some simple steps, so that the rbtools package recognizes your command.
In /src/rbtools/, you’ll need to enter your new command in the array called “rb_commands.” This is where you define the location and class of your command, along with its actual name.
For ex. if patch was the name, type something like ‘patch = rbtools.commands.rbpatch:Patch‘.
After you add a new command, you’ll need to update the environment by running ‘python develop’
Inside /src/rbtools/rbtools/commands, most of your work will be done here. has the base command class that all commands will inherit from. This class is useful for taking care of options, arguments, error handling, and etc.
I would recommend taking a look at Steven’s script to grasp a good understanding on how commands are setup. I would even copy it and setup my new command based of that.
Let’s examine’s main points of interest. Each command that inherits Command will have an “entry point” or a place of starting. This is the method main(self, *args).
The main method takes in arguments from the command line.  Tip: You can actually override the arguments  main(self, my_arg_1, my_arg_2, *args) to force/require arguments. So say the cmd is called “rb patch” but you want to require a request id, the user will then need to type “rb patch 3242” if main looked like this main(self, request_id, *args) 
Options (not to be confused with args, which are required, not optional) are defined in the options_list. The tricky part is to know which options are needed for your command. Defining or modifying options should be straight-forward as seen by’s examples. In the __init__ method, you’ll also have the ability to obtain runtime defaults for options. Be sure to correlate these options with the options you’ve defined in options_list.

Interacting with WEB API

For certain commands, you’ll most likely want to connect with the RB Server, perhaps download a diff/patch file or interact in some other way. You’ll be using the WEB API, though a layer of abstraction, which is good because you won’t need to be sending raw http RESTful requests. Setting up the RBClient can be found in This is the object that will allow you to access the api to do all kinds of cool things. Check out to get an idea of the things you can do with the API. Items and resources are organized in a tree structure. So, let’s say you have an instance of RBClient, called it rb_api. Then you’ll access the ‘root’ of the tree with rb_api.get_root(). With the root, you’re open to the whole tree of resources/commands. Let’s say you want to get a patch file for a specific request and revision of that patch. You’d type something like self.root.get_review_requests().get_item(<request_id>).get_diffs().get_item(<patch_id>).diff . This type of structure can be derived from the aforementioned link which displays the tree structure you can traverse to get to your goal.

Useful Links!topic/reviewboard-dev/HFm7rz4QE2k/discussion!topic/reviewboard-dev/MLOFW3DqoaM/discussion

Meeting Minutes for Nov 4th, 2012


(ChipX86) I hope everyone saw David’s e-mail about Testing Done. This is very important. Treat that field like it’s part of your code. Something to spend time on. We won’t commit things that we’re not sure have been tested well and actually go through every step you outline. Don’t write what you think you’d have done we get patches from other contributors that say they’ve tested, when it’s clear the code has never worked. Keep that field up-to-date as you make changes to your code, and go through the testing each time unit tests will help us to validate that your code is good too.

(ChipX86) The review UI code many of you have had to hand-apply is now in master, so you should be able to just update master now and merge it into your branch. You also are going to need to upgrade your djblets. Some non-backwards-compatible changes had to be made.

(Smacleod) It really helps me to review revisions to your changes, when you provide a good description of what has changed. As you are updating your diffs, use the box that allows you to specify what has been updated, to explain your changes in detail. Keep the description as something we can use for the final commit

Questions / Answers / Tips:

Q (Johns187): For rb patch, I imagine I’ll need to test against other SCM clients.  For example, SVN has the notion of a ‘basedir’ that helps me find the proper pX argument for patch. Can someone explain a good way of testing a bunch of different clients (SVN, Peforce, Mercury, etc)? And do you have any tips on obtaining the correct -pX arg for rb patch?

A: Best way is to create repositories and try them out. SVN and Mercurial are easy to get going. Perforce too, actually. You can get a free download. In Perforce’s case, you’ll just be using plain ol’ patch though. ChipX86 said he’ll talk to a friend about getting the right -pX argument. You may have to accept the arg via the command options. Focus on Git, SVN and Mercurial. Perforce doesn’t work like the others.

Q (Allyshia): When using a 3rd party tool as a part of a plugin (in my case for ReviewBot) like JSLint or any other, how do we procede for keeping those libraries up to date? What do we currently do with the PEP8 static analysis tool?

A: Pep8 is python itself, so it can be managed using the regular packaging stuff for python. As for jslint, we’re just going to have to update it manually, and release new versions of the tool.

side note: This brought up an legality issue with JSLint if it was packaged with RB. RB’s activity may qualify as evil and thus conflicting with MIT plus no evil clause. As a result, we’re going to break JSLint support off into a separate python package, which will be installable separately

Q (Yangtina):1. What’s the general practice for marking issues opened in review request as resolved/fixed? Can we just use them as checklist and mark them as resolved as we code or do we have to wait till we submit a new revision that actually addresses those problems?

A: Definitely use them as your checklist, super helpful.

Q (Yangtina): 2. For testing, is there general rule of thumb for what kind of changes need test cases to be implemented and what kind just need manual testing?

A: If it’s something that can be easily unit tested, you should do it. Things that manipulate data, for API calls, for example, are easily testable. Right now that’s all Python. We don’t have JavaScript unit tests in just yet, but I’ll be doing that pretty soon here. Some things are harder to test – like UI. I don’t believe we’re on the Selenium boat yet. If something looks like it’s ripe for automated testing, you’ll hear about it in a review.

Q (Tahnok) : How should password storage work? I will need to ask the user to input their buildbot username/password (unless they have ssh passwordless auth setup) but I want to make sure I’m storing that information properly

A: We want to keep as much configuration as possible in the Review Bot admin panel on the review board side. An option is to look into AES encryption using pycrypto (which is already a dependency). It’s two-way encryption. You can tie it to the settings.SECRET_KEY.  We should encourage ssh-based access. The issue here is, the creds must be sent through the queue, the the worker, which doesn’t have the secret key. The queue *should* be secure.  I think we’re going to have to have some sort of on-worker-machine configuration for this unless we just store and send user/pass in plain text. This is a tricky problem and will be discussed outside the meeting.

Q (Tahnok): We’re going to be including buildbot as a dependency for reviewbot?

A: No, it should just talk to buildbot, and so Review Bot has two pieces as well.  The extension shouldn’t have any knowledge of buildbot. buildbot try is a command line utility like post-review and thus we’ll have to have it as a dependency then.  can handle the buildbot dependency.

Q (slchen) I realized a potential problem with truncating .rst and .md files before passing them to docutil / markdown APIs: Truncating after the first 20 (or 5) lines can cause certain structures (such as tables in RST) to become ill-formatted / syntactically invalid when they are passed off to the docutil / markdown APIs.
The docutil API handles it as follows: 1) output the ill-formatted table in a <pre> block, 2) append an error message immediately after this <pre> block. In the sample .rst and .md files this hasn’t caused a problem yet, because the cropping area of the thumbnail is only about ~10 lines, so you never see the error messages farther down the html.
It’s a sticky situation, because neither of the libraries have APIs for parsing the text-ish files one “structure” at a time, so doing it dynamically (i.e. truncate the file after the last “structure” past a certain line #) would require adding to docutil / markdown APIs (which is something I assume not worth our dev time investment).
My proposed solution is as follows: set a higher upperbound for the truncation (truncate after 200 ~ 300) lines – which still leaves a constant processing time on converting the textish files to html through the APIs, and we can reasonably assume that in 99% of use cases people won’t upload a .rst file with a table that spans 300 lines.
but it’s a design decision – can we do better?

A: It’s fine, though I am curious what benchmarks look like for rendering such files. You can use our log_time stuff around the rendering code to measure (examples in A nice improvement would be to stick the rendered results in memcached so that way it’s only rendered once, optimistically. Also have a look at cache_memoize.

Side note on extensions and hooks: Hooks has some good documentation. Check out: /, ,

Other Notes:

  • ChipX86 will be away or less active in the next couple days. He’ll be most likely unavailable in the next two meetings. He’ll be active during usual times except this 10th through 12th he’ll be out during the day, and the 15th through 19th he is on vacation and will likely only poke my head in for short times in the evenings.

GUI based IDE and useful plugins for better code

I’m terrible with VIM (or console based editor like emacs) and come from a Windows GUI background. So my VIM chops and console skills are abysmal.

Luckily I found an awesome GUI based IDE for Python called Geany. I like that it has a terminal built in (some things you just can’t get away with), a compiler to run your Python code, clean layout, syntax highlighting for Python, etc. It also saves state of your opened files, so you can resume your work just by opening the IDE again.

What’s even better is that you can add some plugins that will help you write better code. Pep8 and Pylint can be integrated into Geany so you ensure your code meets standards. Learn how to install it here. I was constantly submitting bad code, such as trailing spaces or poor indentation. This fixed it.


Don’t forget to enable ‘Show White Space’ under ‘View->Editor’ and change default indentation to spaces.

GIT UI Visualization – Very cool tool called called GITG Kind of neat to have. May help those new with GIT.

“gitg targets cases where it is useful to provide a graphical representation of Git data or actions.”

I hope this helps future RB’ers that are new to Linux environment and Python development.

UCOSP Blog Post

Intro: My name is John Sintal and I am a 4th year student at the University of Toronto (U of T) studying Computer Science.

What I am working on: I am working closely with Steven Macleod on building a set of Review Board (RB) tools, individual scripts that work as commands for Review Board. An example of this type of command would be a diff command. A developer would like to view a ‘diff’ in his terminal window, and so he would type ‘rb diff’ to view it. Overall, Mike Conley described the project as deconstructing a “monolithic” code base, referred to as post-review, and adding new functions. Ideally, the new commands that are being built will soon replace the majority of post-review.

My thoughts: I am very fortunate to be in this program because it is definitely bringing some great hands-on experience with software development, something that is very important to me. For instance, picking up GIT and becoming more comfortable in a Linux environment are some new things I’ve learned just from the experience of this project. Throughout my time at U of T, much I’ve learned with regards to computer science has been theoretical. As much as theory is important, I am more interested with the application of software development.

The people and the positive team vibe are reasons why I am enjoying this project course . Everyone here on the RB team is extremely friendly, hard working and shares similar goals of contributing to this open source project.The mentors are especially helpful, down to earth and friendly. Near the end of the code sprint, it was a bit sad to say bye to all my new friends.

Some of the hurdles I have encountered in this course so far is managing my time, asking for help when appropriate and overcoming a rather steep learning curve. Time management is key to success in this course. The amount of freedom you have is something you must carefully deal with. It’s very easy to not put in the honest hours of work because you don’t have strict deadlines or constant reminders. It is your responsibility to establish an effective schedule and stick to it. Communication or asking for help/advice is another important factor to having success in this course. For instance, at the sprint I was looking at a bug for a couple of hours, wasting time by not asking for some help. Had I been a little more proactive and asked for help, I could have save more time. In terms of learning curve, I come from an Eclipse IDE GUI Windows background and working in terminal driven Linux environment was a major learning curve for me. I felt really behind, but I’ve learned that it takes patience and persistence to overcome these struggles. Now, I am feeling more comfortable in a Linux environment.

That’s all for now. Review Board is awesome! The people behind it and the tool itself. I look forward to using it more in the future.

Postreview, new web api, synctransport diagram

I am a visual learner and was fortunate enough to have Steven (one of the mentors) draw on the whiteboard a visual representation for the stuff I will be working on, and some of the stuff Tina will be working on.

Clicking on the image should give you a description and my best, succinct summary of what is going on. The 2nd image on the synctransport is quite complex and thus I am not totally sure what’s going on. Please take my description with a grain of salt. The pictures are mainly here for reference in case other people would like to have a look. I may update this post if/when I get better understanding of the complexities.