Author Archives: tnyang

Review Board Models Visualized

visualization of reviewboard's models

visualization of reviewboard’s models

Generated by django-extensions’ graph_models, this is based on the master branch on 02/12/2012 plus the inline file attachment evolution.

Although this is not the most concise or human friendly graphical representation, hopefully this overview could be of some assistance to you.

Advertisements

TL;DR Useful Tips All-In-One Index

There are lots of useful information about review board out there but sometimes it’s hard and costly to dig for them. Wouldn’t it be nice if there’s an index for them to save all the random access cost to search for them?

Well, here it is. If we all contribute to this list with tips or links to useful information, then this can be very useful for newcomers to review board and a great ‘how-do-I-do-that-again’ reference for everyone.

Getting Started on Review Board

  • Apply a patch the git way
  • 1. download the diff (from a review request etc)
    2. put it in the rb repo, then:
    3. $ git apply rb****.patch
  • Apply a patch the cool johns187 way
  • $ rb patch <rid>

Testing Review Board

  • To test review board in local test environment
Add new repository on localhost:
name: Review Board
hosting service: GitHub
project owner: reviewboard
project name: reviewboard

post-review --server=http://localhost:8080

Link to post: https://reviewboardstudents.wordpress.com/2012/03/03/how-to-set-up-local-test-environment-for-review-board/

  • To run a specific unit test case (subset of test cases):
./reviewboard/manage.py test -- reviewboard.scmtools.tests:GitTests.testFilemodeWithFollowingDiff

Link to post: https://reviewboardstudents.wordpress.com/2011/09/24/running-subsets-of-review-board-tests/

CSS tips

  • Use the 4 CSS rules of multiplicity well
1. Multiple declarations can live in a single rule.
2. Multiple selectors can preface the same rule set.
3. Multiple rules can be applied to the same selector.
4. Multiple classes can be set on a single element.

Link to external post: http://www.cssnewbie.com/css-rules-multiplicity/

  • Good practice: Use the multiplicity rules instead of mixins (less css)
  • "Mixins are neat and all, but my preference is to avoid them in cases like this (when you want two classes of basically the same thing but only a slight difference, such as one floats left and one floats right). The code should be:
    
    .class1, .class2 {
       // All the common stuff here.
    }
    
    And then have the .class1 and .class2 rules define their own specific stuff (like their own floats and 'a' rules).
    
    The reason is that, while it's neat to be able to include data like this in lesscss, in the end it's really just turning into even more CSS code the browser has to load in and keep separate, and it's more work to debug. It's equivalent to copy/paste.
    
    Similarly, this situation (class inheritance) shouldn't use a mixin, as it brings in a lot of code. Instead, the elements using the various "child" classes should be also specifying "parent" as a class. More reuse, less debugging and less output."
    
                                                                      - ChipX86

We are using LESS for reviewboard, more about LESS: http://lesscss.org/

Status Report — Nov 17th, 2012

Wesley Ellis

Currently:

  • Writing the settings panel for buildbot

Roadblocks:

  • Can’t get the buildbot try command to accept diffs
  • Cant’ get buildbot try to connect via ssh

Next:

  • fix roadblocks

Jesus Zambrano

Currently:

  • Changing inlineEditor so that disable editable fields when a review request is discarded.
  • Fixed the issue of more than one pencil icon on the discard-banner and submitted-banner

Roadblocks:

  • Nothing major, just somewhat confused as to how the inlineEditor works.

Next:

  • Add a field to RB.ReviewRequest to check if a review request is public or a draft. This would help display the appropriate banner(or no banner) when a submitted review request is re-opened.

Allyshia Sewdat

Currently:

  • Updated WIP review request for JSLint plugin with ‘final’ submission pending any change requests. For now this looks like the final state in which I will leave that particular project for the term, not counting potential polishing later on. The WIP status could be removed as soon as some additional conclusive testing is completed to confirm performance quality, even though functionality has been proven.
  • Got some feedback on mockups for the manual trigger of static analysis.
  • Looking into the codebase to figure out how to hook up the new manual trigger functionality.

Roadblocks:

  • None for now

Next:

  • Continue the exploration of the codebase to come up with a sound design for the manual trigger, taking into account the tips given to me by mentors.
  • Will continue to accept feedback on the WIP review request for JSLint plugin and make adjustments.

Questions:

  • Any immediate tips or gotchas that come to mind regarding implementing a new navigation bar option / implementing the ReviewBot manual trigger extension would be appreciated. Otherwise I’ll keep pinging people for guidance.

Aamir Mansoor

Currently:

  • Took Markdown Pluggable UI out of WIP. It can be reviewed at r/3434
  • Looking into loading pluggable UI in a lightbox rather than redirecting to a new page

Roadblocks:

  • None

Next:

  • Continue working on rendering pluggable UI in lightbox

John Sintal

Currently:

  • Finish rb patch and addressing David’s issues

Roadblocks:

  • Can’t eliminate certain options due to complaints in legacy code
  • Trying to figure out pX logic, but the base dir given by repository_info is always “/”
  • Figuring out how to setup SVN

Next:

  • fix roadblocks

Questions:

  • I’ll be posting a new patch very soon with regards to my road blocks. If anyone can help me eliminate the superfluous options or give pointers on how to deal with the legacy code that expect these options to exist when obtaining a SCM.
  • Do we have a testing repo for SVN?

Karl Leuschen

Currently:

  • Monitoring my active review requests and making changes as suggested
  • Submitted blog post for the week
  • Looking into Bitbucket API

Roadblocks:

  • Need someone to take a look at r/3488. It’s not stopping me from having work, but I’ll need some help on it in order to finish my project’s core feature.

Next:

  • Work on finishing WIP patch (r/3488)
  • Start getting connected with Bitbucket
  • Look at more open review requests

Tina Yang

Currently:

  • Working on resolving a CSS issue on Chrome only for inline binary file UI and cleaning up code for code review.

Roadblocks:

  • None.

Next:

  • Upload binary files via post-review (Git)

Sampson Chen

Currently:

  • Working on ReviewUI integration with Extensions. Making progress, should have a WIP up later today.

Roadblocks:

  • None, just been crazy busy lately

Next:

  • Add in changes for memcaching for thumbnails.

Michelle Chuang

Currently:

  • Addressed code review comments for r/3458
  • Working on creating a view with multiple image diffing options
  • Working on image diffing through a “swipe view”

Roadblocks:

  • None

Next:

  • Continue to work on different image diffing views

Meeting Minutes for Nov 11th, 2012

Announcements:

  • We have 3 more official meetings before we wrap!

Questions / Answers / Tips:

Q: (Karl-L) Should we make sure to plug the UCOSP program in our one required blog post?

A: The goal of the blog post is not really to plug the program so much. The stated goal is to show people outside of the program, including prospective students, projects, and sponsors, what it’s like inside the program. Just tell people what you’ve experienced so far, what you’ve been running into, etc. So really say whatever you want about it. 😉

Q: (John) ChipX86 said he was going to ask a friend for pX argument logic

A:  (ChipX86) The pX logic doesn’t actually exist. I was misunderstanding what he did. So you’ll have to compute based on the RepositoryInfo’s base/root path, current directory, and base path on the ReviewRequest from the server

Q: (purple_cow)  re: rbtools commands: in the diffs there are a bunch of commandline args that don’t apply why are those still there?

A: The SCM requires that they exist.

F/U: (purple_cow)That’s something you (John) should probably put some focus into, refactoring things so you don’t need to require those arguments.

Q: (Allyshia) I ran into a weird bug with ReviewBot which slowed down my review posting last week (I sent a message to Steven about it, so we can talk offline later). It doesn’t interrupt my plugin’s functionality but it does make it output less than it could. I’m going to post the new proposed fix (that might be temporary) to a review request right after this meeting and my goal is to focus a lot more on my larger project for the rest of the time. I figure that it’s best to leave the plugin with room left to polish but contribute more to a more interesting project. Does that sound right to you?

A: That sounds fine with me. You’ve taken the JSLint plugin to a good point so far, and you really are running low on time for the other project

Q: (aam1r) I am not sure what to work on next

A: “File attachment review UIs in a lightbox” – there’s a few things in ReviewUI and in the template that are the beginnings of that, but nothing is at all hooked up. Applies to anything that specifies it can be reviewed “inline”. Images being a good start. Ideally, we can flip a flag on the ReviewUI subclass and get a lightbox. There wouldn’t be any diffing. So it’s not intended as being for binary files uploaded with a diff, which later we’d want optional diffing abilities for. We may decide we want that (like md) inline, and it’s good for testing. The ReviewUI subclass shouldn’t have to do really anything different to be in a lightbox. This is more about the general infrastructure around ReviewUIs.

Q: (yangtina) When is the hard pencils down deadline for this term?

A: I believe we’d settled on a soft pencils down around December 4th. The hard pencils down will likely be when the steering committee asks for evaluations to be due. It will be likely a few days after December 4th.

Q: (tahnok) smacleod, is there some way to have settings fields that show up depending on an option?

A: I doubt that will be possible with the current system. You might have to try and use a single form field that incorporates all of those options, or just tell the user certain options are ignored. (To give everyone a better idea, Review Bot tools can specify an option attribute on their class which is used to create a form on the RB side. RB doesn’t actually know anything about the class though, so this options attribute is a dictionary specifying django form fields as strings. It gets serialized and sent across to be stored in the DB)

Q: (slchen) are there projects we can work on if we are interested in sticking around after UCOSP? I’m finding that I’m doing more learning here than in the classroom b/c it’s almost all exploratory. Having mentor feedback helps a ton.

A: There is always work. *always* and sticking around after the term ends is highly appreciated / awesome. 🙂

Other Notes:

  • ChipX86 is going to be especially busy for the next few weeks
  • Students, watch the calendar carefullyover these next couple of weeks. Don’t get stuck. Don’t forget to ask questions. Get unstuck.
  • m_conley will get a firm pencils down date from the steering committee and send it out via email
  • We’d be absolutely happy to have you all continue hacking on RB after the course ends. We’re growing an awesome community here, and it rocks when people stick around. Plus, we’re going to have another crop of students come January who are going to need help from people who know their way around the code / process.
  • When you’re a volunteer contributor, you just do your best when you have the time. 

UCOSP Blog Post

Hello everyone, I am Tina Yang, a Computer Science student from the University of British Columbia. I love the idea of open source and group collaboration so I did not hesitate at all when applying for the UCOSP program. This program, specially the Review Board team, has lived up to or perhaps even surpassed my high expectation of it.

We have been following an organized schedule to push forward in the project. Here is an (my) idealized week of a student working on the Review Board project to demonstrate our workflow and the tools and process we utilize:

Monday Review the meeting minutes (summary of last week’s irc meeting) and plan out the goals to achieve this week.
Tuesday One of the students in the group will submit an UCOSP blog post.
Sync local code with master using Git and start hacking away.
Wednesday While working through the code, find a process tedious/difficult to do, search through Review Board’s Google Groups and Blogs for tips and discover some clever tips/script on the blog that makes your life a little brighter.
Thursday Unable to solve a problem after decent hours of debugging and googling around, decided to paste the error in http://pastie.org/ or http://pastebin.com/ and ping mentors on IRC to consult them on the matter. The mentors patiently give hints and advices regarding that problem and some helpful peers (students from the current or previous terms) join in the conversation sharing their experience with similar problems.
Friday Run the newly written code through the PEP 8 style guide script. Merge from master and run all the automated test suites to make sure they pass with the new code. Feeling good about the new changes and submit them to Review Board for code review. (small chunks of code are better!)
Saturday Submit a status report highlighting what has been done or is in progress, what were the roadblocks, what is next, and any possible questions 24 hours before the IRC meeting on Sunday.
Sunday Prepare questions for the IRC meeting.
Participate in the one hour IRC meeting and wait for your turn to ask the mentors questions.
One student in the group will compile the important points in the meeting into a summary and post it on the blog as meeting minutes.

I feel really privileged to be in this program, and the friendly and intelligent group of Review Board with experienced and patient mentors and motivated peers. I’ve had a great experience so far and hope that we will all have a blast this term!