Author Archives: Mike Conley

[Open Academy] Status Reports: May 2, 2015

Jessica Qian

  • What project are you working on?
    • Trivial publishes
  • What you accomplished this week
    • Fixed bug with trivial being True.
  • What you plan to do next week
    • Work on final report requirement for class.
  • What, if anything, is blocking you from making progress?
    • None

  • Any other questions
    • None

Teresa Fan

  • What project are you working on?
    • Better Linking in the UI
  • What you accomplished this week
    • Changed the link icon to be in a different place (now inline with the comment text, and right-aligned)
  • What you plan to do next week
    • Adding link icons for replies too?
  • What, if anything, is blocking you from making progress?
    • Nothing. Reviews are always welcome
  • Any other questions
    • Nope

Viet-Tran Nguyen

  • What project are you working on?
    • Pre-post hook and uncommitted flag
  • What you accomplished this week
    • Address some left over comments for the above issues
  • What you plan to do next week
    • Work on final report requirement for class.
  • What, if anything, is blocking you from making progress?
    • None
  • Any other questions
    • None

The relationship between ReviewRequest and ReviewRequestDraft

Despite having worked on various parts of Review Board for several years, the distinction between a ReviewRequest and ReviewRequestDraft has often been a bit blurry for me. I’ve had the occasion recently to explore the relationship between these two models, and I wanted to write it down for future code-spelunkers.

The way I want to explain this is by exploring the lifetime of a ReviewRequest, from cradle to grave. So, let’s get started.

When a ReviewRequest is first created, it has barely information in any of its fields, and the public attribute is set to false. When the user starts making modifications to a Review Request though, a Review Request Draft is then created. The changes that the user makes after creating the initial Review Request actually update the Review Request Draft instead. Even if the Description and Testing Done fields have been populated, that information is not in the ReviewRequest: all fields are set on the ReviewRequestDraft.

When the user clicks “Publish” on a review request page, the information in a ReviewRequestDraft is dumped into the ReviewRequest model. The ReviewRequestDraft model is deleted, and the state on the ReviewRequest is set to public. Congratulations, your review request is now visible!

Suppose now you want to make a change to a review request. Perhaps you’ve updated the Testing Done field, or you’re adding a new reviewer to it, or you’re changing the summary. What happens is that a new ReviewRequestDraft is created for the ReviewRequest, with a copy of all of the current fields in the ReviewRequest, and any changes you make go into that ReviewRequestDraft.

So when you, or anybody who has mutation-privledges for a ReviewRequest sees the review request page, and a ReviewRequestDraft exists, you see the information in the ReviewRequestDraft. For anybody without mutation-privledges looking at the page, they see the unchanged ReviewRequest data.

When it comes time to publish the draft, the changes in the ReviewRequestDraft get, again, dumped into the ReviewRequest (a ChangeDescription is also created in order to document the changes being made to the ReviewRequest) and the ReviewRequestDraft is destroyed.

When a ReviewRequest is marked submitted or discarded, that state change occurs on the ReviewRequest, and ReviewRequestDraft does not need to be created.

So that’s how changes occur on ReviewRequests.

Meeting Minutes: September 7, 2014

Introduction

Welcome! Congratulations, you’ve joined the best team.

How things generally go in these meetings:

  1. Opening announcements
  2. Round-robin to give each student time to ask questions
  3. Closing announcements

m_conley is posting meeting minutes this week (and posting them late – sorry).

These meeting minutes are going to be pretty extensive. Most minutes should look like this.

Mentors

  • ChipX86 (Christian Hammond)
  • heapify (Anselina Chia)
  • m_conley (Mike Conley)
  • purple_cow (David Trowbridge)
  • smacleod (Steven MacLeod)

Students

  • andrewhong (Andrew Hong)
  • asalahli (Azad Salahli)
  • brennie (Barret Rennie)
  • dkus (David Kus)
  • justy777 (Justin Maillet)
  • mloyzer (Mark Loyzer)
  • nicole_xin (Nicole Xin)
  • rdone_ (Ryan Done)

andrewhong and nicole_xin are absent this week due to traveling.

Getting going

Your primary goal right now is to get your development environment set up, and if you’ve got it set up, then you should be looking for EasyFix bugs to fix. Claim a bug by commenting in the bug saying that you’re taking it. Don’t take any bugs that have been claimed by other students from this semester. If you see a comment claiming an EasyFix and the name isn’t in the list above, it’s probably fair game (feel free to ask if you’re not sure).

EasyFix bugs are your onramp, and then we’ll get you assigned to some larger projects. Here’s our current project list for your perusal.

Projects are worked on by one student at a time – we generally do not put students on teams on the same project, as we find these hard to evaluate.

Project selection will probably occur at the sprint.

OMG Ask Questions. Also, we’re human.

This cannot be overstated. If you want to survive this course, you need to ask questions when you’re stuck. If you are stuck, if your are immobile, do not just stew there hoping the solution to come to you. Try a few things, and if you cannot figure it out, just ask one of your mentors in #reviewboard-students. We’re here to help you and answer your questions. Use us. Help us… help you. Help us… help you.

Also, we’re human. Don’t be afraid to talk to us. We’re friendly. Don’t be intimidated by us. We also make mistakes and get stuck and ask each other for help. Ask questions and you will not be thought of as silly or foolish. Just ask!

We’re here to help you. We are friendly people who love to help. It’s our job to help you, and we like that job! Let us do our jobs and ask!

Also, don’t wait until meetings to ask us. Ask us when you’re stuck. “Ping” us in IRC.

Start by asking in IRC. We might be AFK, in which case, we’ll respond when we get back (this works really well if you have a bouncer – ask one of us to get one). If you’re trying to get feedback, the reviewboard-dev mailing list is a good idea – that’s the mailing list for the wider Review Board development community. They’re good folk.

UCOSP / admin stuff should go to the reviewboard-students list.

Direct email or private IRC to one of us mentors is a last-resort sort of thing, and should be for things that you want to keep private. Otherwise, put it in the open – that way, us mentors don’t get out of sync.

Other questions

Q: Do you have a link to a django tutorial? or documentation that would help us get going?

Yes, here ya go. Tutorial is in there too. Make sure you’re reading the right version (check bottom right corner – current is 1.6). Also, check out the blog posts that have been written by past students on this blog. Your Welcome Packet linked to some decent posts.

Q: What version of Python does this project use?

Python 2.6 and 2.7 for Review Board. RBTools runs on Python 2.5 – 2.7.

2.7 is a good bet for your work, just don’t do anything 2.7-specific.

Q: Are there any plans to support Python 3?

Yes, we’ve done initial porting work, but not much testing. We’ve gotta move with the dependencies of our install base, so as more people adopt Python 3, we’ll take supporting Python 3 more seriously.

Q: Are there documents anywhere on code organization? Especially the Javascript bits?

We don’t, no. 😦

We re-wrote the entire JS codebase for 2.0, and we’re still shuffling things around.

When exploring the codebase, maybe take notes which could help seed such documentation.

Q: Do we post WIPs as review requests?

Yes. Please prefix the summary with [WIP]. Any review requests you create should be in the “students” group.

Q: Is the django (or the rest of the code base) in the same docs situation?

Sadly, yes. We do have a lot of documentation inline – docstrings in classes and functions. We’ve found that documentation that’s not part of your codebase gets out of date very quickly – and out-of-date / incorrect documentation can be worse than no documentation sometimes.

We might give a whirlwind tour of the codebase during the sprint.

Djblets and Review Board are divided into modules (“apps” in Django terminology), and are named in ways that help you locate where your code belongs – “diffviewer”, for example.

Git grep is your friend.

You’ll get familiar with the parts you’ll start working on, and we’ll be able to direct you to the right places if you’re unsure.

Q: Do we add ourselves to AUTHORS?

Feel free to do that with your first change.

On project specifications

As software development students, you’ve all probably written a bunch of code for courses, and the specs you’ve probably written that code from have probably been pretty well defined.

This course is different. Review Board is a real product used by real users. We have to be pragmatic about projects. Pragmatic means coming to terms with the fact that things change, nobody can predict the future accurately, and nobody is perfect.

You might hit a deadend with your project that we didn’t foresee. It’s not super-common, but it happens. If that happens, we’ll move you onto a different project while we take the spec back to the drawing board. That’s just how the industry works sometimes.

Because some parts of the spec may be vague on our part, you may get really excited about a direction you want to take the project. That’s a very good thing, but please run it by us first. if you encounter a problem with the design or some unknown, and have an alternative way you want to go about it, talk to us before you do too much work on it. sometimes what may seem challenging has a simple solution, and sometimes a different direction will actually conflict with something else that’s going on elsewhere in the code.

The weekly meeting is a great opportunity to stay in sync with what the other students are doing to make sure what you’re doing gels with them.

Misc

  • Review Board is not a traditional Django project. We use a lot of Django conventions in some places, but in other places we completely alter how Django works for our purposes.
  • Hackpad is where you can take public notes. Get yourself a free account there. When you work on a project, we expect you to keep a Hackpad with notes. File any pads you create under the “Students” category.
  • Some EasyFix bugs might turn out not to be so easy. That’s still useful information, but come talk to us when that happens.
  • We’ll give you a t-shirt at the sprint if and once you’ve put up your first review request!

 

Status Reports: Sept 7, 2014

Barret Rennie

  • Got development environment set up
  • Ran into slight issue: reviewboard package requires ‘django-evolve’ version >= 0.7.4.dev, but the PyPI only has version 0.7.3
    • django-evolution is part of the Review Board family of modules. You’ll want development versions of it, Djblets, Review Board, and RBTools.Sometimes you’ll pull the latest changes into your Review Board tree and get some breakage that that you didn’t see before. When that happens, go update django-evolution and Djblets, as development often happens across these modules that the other modules will depend on.
  • Did a bit of poking around the code base and the EasyFix bugs list. I submitted my first review request for an open bug.

Ryan Done

  • Got development environment set up on an Ubuntu laptop for the sprint, and a vagrant VM on Windows for working from home.
  • Encountered the same django-evolution issue that Barret did

David Kus

  • Got development environment set up on OS X using vagrant + VirtualBox
  • Played around with the demo a bit and explored the code base a little, getting used to Python and Django.

Nicole Xin

  • Got development environment set up on OS X using vagrant + VirtualBox
  • The instruction is very helpful but I did run into one issue that vagrant cannot find provider vmware_fusion so I use virtualbox instead. So far I cannot find any solution to this problem.
    • You need to get a license for the VMWare provider for vagrant. We can sort that out at the sprint or sometime after – it shouldn’t be a huge blocker.
  • For most of the links on the demo page http://demo.reviewboard.org, I received Error 500. And rest of them are almost empty(blanks everywhere). Is it just me or something went wrong?
    • Youch! How embarrassing. That should be fixed now.
  • I’ve been looking through the code and the EasyFix bugs list. I’ll try to work on some bug tomorrow.

Justin Maillet

  • Got development environment set up
  • Ran into django-evolution problem, but got it sorted over IRC
  • I have been exploring Review Board trying to get a handle on how the code is organized, what it does, and how it all works together. I’ve also been brushing up on python an django. I’ve also started poking around the easy bug fixes.
  • What is the release schedule like for Review Board?
    • “When it’s ready.” We don’t have a set schedule. Instead, if we feel a release is ready (has some features we want to expose, or some important fixes, or any security fixes), we’ll release.
  • What other projects use Review Board?
    • There’s an estimated 3000+ companies using Review Board. This includes companies like Twitter, VMware, Yahoo, Cisco, Mozilla, and many other large names that we can’t list here (due to not having permission to publicly post yet), but can discuss in private.The KDE desktop environment on Linux also uses Review Board for code reviews, as does the Apache foundation, OpenH264, and others.
  • What will be happening at the event on Thursday?
    • Thursday is just a social event to meet people and get to know them.
  • What will the code sprint be like?
    • At the sprint, most of our time will be spent working on code. For anyone who hasn’t yet finished their dev setup or easy fix bugs, we’ll start with those (though you should be trying very hard to get that done before the sprint to optimize your time!). After that, we’ll all choose projects for the term and start in on those. At some point during all this, we’ll probably do some short talks about Review Board’s architecture and processes, and we’ll do a group code review session to demystify peer review.

Andrew Hong

  • Got a Linux Mint VM set up, but Review Board development environment has not yet been set up on it. Will do so when I get home on Monday
  • Been offline for a few days, and flying back to Toronto today (subsequently, must miss first meeting)
  • Have set up XChat on my VM, and waiting for a bouncer

Azad Salahli

  • Have not yet set up development environment or investigated Review Board, as I have just returned from a long vacation.
  • Will be attending the meeting.

Mark Loyzer

  • Struggling to get development environment set up – I had an issue with vagrant not being compatible with VMWare Fusion, so I had to switch to VirtualBox.
  • I’m having the same django-evolution issue as reported by Barret and others.
  • Looking to start some EasyFix bugs ASAP.

Status Reports: May 18, 2014

Tami Forrester

1. What you accomplished this week

  • I’ve been working on finishing up the issues from the latest reviews for /r/5484 – only a few issues left and I’ll be done! That should be completed today.
  • I was also working on adding notation to the diff viewer and figuring out how that worked (for this issue), though that’s been on pause for a bit. My main approach for this is to get the latest reviewed diffset on the Review_request, and send that as a variable into the javascript object DiffViewerPageView that will then handle displaying it. A WIP will be up for it soon!

2. What you plan to do next week

  • Finals! I’ll be back after those finish up.

3. What, if anything, is blocking you from making progress

  • For the diff viewer, my main plan of action is getting the last reviewed diffset number from a user’s review on a request. As of now, this entails searching through each diff_comment from a user on a review request, and then returning the latest diff number that was reviewed. This isn’t really a blocker, but I’m wondering if there was a better way to do this, like having a latest_revision field on each review. I noticed that reviewed_diffset (Diffset), has been deprecated on the Review.

4. Any other questions

  • None

Peter Tran

1. What you accomplished this week

  • This week I worked on a bug fix. I added delete functionality to sessions and fully tested it. The code review was submitted and corrections were suggested. I’m currently adapting these corrections and sending it in for a (hopefully) final review.

2. What you plan to do next week

  • This week I plan to start my project. I think my first step is to architect and test a notification system.

3. What, if anything, is blocking you from making progress?

  • What’s stopping me from making progress would be my lack of knowledge of the codebase. I’ve poked around the codebase docs however there isn’t really any architecture information. I’m assuming I can get a handle on where to hook into around the accounts directory.

4. Any other questions?

  • Question: Can I create my own models and test cases or should I be extending an existing model/objects?

Volodymyr Lyubinets

1. What you accomplished this week

  • Fixed 2 bugs, one was a quick fix, the other not so much (r/5810/ and r/5797/)
  • Wrote a proposal of my project (https://reviewboard.hackpad.com/Reviewing-attachments-tbfh4oiwTHB). Any feedback is welcomed.
  • Explored the code that might be relevant for my project. Now I know where the relevant templates are located, what database table is used, and have an idea of how to proceed.

2. What you plan to do next week

  • Start writing some code and publish my first review related to this project.

3. What, if anything, is blocking you from making progress

4. Any other questions (could be project related, course related, or industry related. Anything goes, I guess).

  • None

Raheman Vaiya

1. What you accomplished this week

  • This week I fixed bug 2766 and resolved to work work on dnd image
    support for the review editor view. To this end I started looking at
    the relationships between different parts of the code base including
    url resolution, views/templates, and backbone/django models.

2. What you plan to do next week

  • Start thinking about backend and front end changes that need to be
    made to accommodate drag and drop support for images on the review
    editor.

3. What, if anything, is blocking you from making progress

  • Most of the technologies used in reviewboard are new to me so I am
    trying to develop a better understanding of their roles in the code
    base. Specifically I am not clear on how backbone and django models
    are related.

4.

  • None at the moment.

Salam Alyahya

1. What you accomplished this week

2. What you plan to do next week

  • Have a clear idea about how the code that is related to my project works. Make a plan or design on how to approach the project, and write some code.

3. What, if anything, is blocking you from making progress

  • I need to get in the routine of working on ReviewBoard regularly.

4.

  • None

Matthew Maclean

1. What you accomplished this week

  • I fixed bug 3074 and have two requests out to fix bugs 3328 and 3329.

2. What you plan to do next week

  • make the requested changes to my code
  • allow the reviewbot to pull the entire repository

3. What, if anything, is blocking you from making progress

  • nothing

4. Any other questions (could be project related, course related, or industry related. Anything goes, I guess).

  • For in code comments, are we to reference the bug number so as to provide additional context or just explain it all in the code?

 

 

Meeting Minutes: March 1st, 2014

Announcements

  • CMU is on break next week, meaning that CMU students are about halfway done the term. Adjust internal calendars accordingly!

Round-robin

Tami

  • A question about the date reviewed and timestamp columns for the review request data grid – they seem to be the same…
    • Yep, they are the same! So we don’t want separate columns for them.
  • General Django question – I notice that a lot of query sets are augmented with more data, and I’m not actually sure what that does… like queryset.select_related(‘some keyword’)
    • When you’re fetching a bunch of rows for, say, Review objects, and all you need are fields that live on Review, then you never need to deal with augmenting.  However, if you ever have to deal with something on another object, say, the Review’s parent ReviewRequest, you don’t want to fetch the ReviewRequest for every row. So this is a way of bundling up requests for both of those object types in the same query, which is faster.

Tomi

  • Is there any examples on how to implement the “Add another” -button shown in the configuration dialog mockup that dynamically creates new pairs of Django widgets to a form?
    • Django doesn’t support that built-in. You’d need to build a widget. Use Javascript to create the form elements, and then loop over the request.POST in the view.
  • Older versions of Gitlab API v3 only allow creating the token with email as the username (Example). Currently review board only posts “login” parameter so older versions of Gitlab do not work. Should I add new input field for the configuration dialog for the email in addition to the username, or change the current one to accept either? If latter, should I try to recognize emails and use “email” parameter accordingly?
    • If you can make it easily accept both, we should do that. For email address recognition, try to re-use the validator that we use for email. That way, we’re consistent with Django. Run the validator and if it comes back as a valid e-mail address, use the email field.
  • Gitlab’s API is really tortuous. 😦 And capability detection is hard.

Stephanie

  • I fixed the two bugs from last week! (The Masonry and event handling ones).
  • Going to focus on the interface for adding widgets now.
  • Maybe after that, we can try to expose some hooks for extensions to add widgets.
  • How long does it take to ship something after its complete?
    • We dogfood almost bleeding edge on r.rb.org. For the changes you’ve already made, they’re likely to ship in a month or two once 2.0 is out the door.

Joonas

  • The mentors are a little wary about using django-mobile to serve up different templates for mobile users. This increases the amount of stuff we need to test, and it’s likely the mobile stuff would get poor coverage and support.
  • We’re rather you focus on using responsive web techniques to modify some parts of Review Board to be more mobile-friendly.
  • It’d be good to investigate some responsive web design techniques. Example. Media Queries are relevant to your interests. Also, Responsive Design View.
  • Start with something small, like the login page.
  • When I run the site / development server inside a VM, how do I access it with a mobile device? When I run ifconfig inside the VM, should I see a wlan0 section?
    • You should just be able to visit your laptops IP address at port 8080. The dev server is port forwarded out to your host machine, so “localhost:8080” accesses it from your laptop or “<laptop-ip>:8080” for other devices on the network

Mirai

  • The mentors really enjoyed the mock-ups!
  • Like Joonas, Mirai will be attempting to use responsive web techniques for some section of Review Board, using his own mock-ups for inspiration.
  • What are the requirements for CMU students over the break?
    • No specific requirements.

Olessia

  • Fixed up the code I wrote last week and wrote some tests, but getting failing tests.
    • ChipX86 will work with Olessia after the meeting to get the tests working.

Anselina

  • Basically done with Git hooks and refining it – just need some reviews!
  • Will be working on GitHub hooks next.
  • Bhushan has a WIP change for the URL stuff you’ll be using. You might want to review it.

Bhushan

  • So the repo URLs are getting added when registered and deleted when unregistered. I tested that using the shell. But in the test case Ive pointed the url to the login page. and I get a 404 error. The regex is not added to the list for some reason.
  • I was thinking of starting work on the hooks…now that we’re close to getting done with this. Should I work on Mercurial next?
    • Sounds good – just coordinate with Anselina.

Iines

  • Haven’t done much – I have to catch up next week.
  • mconley will prepare a more detailed specification to Iines to make sure everything is clear.
  • Iines is confident she’ll have more to say next week!

Edwin

  • Fixed the 500 errors from last week.
  • Investigating why the diff expanding just isn’t working. I might have some questions soon.
  • The diff expand link is like +20. Is that an image?
    • It’s an image – static/rb/images/icons.png and icons@2x.png. Source is icons.svg.
  • I was thinking of for the review page only expanding 5 lines or something smaller so I wanted to have a +5 instead of a +20
    • Let’s keep it at 20 for now, and tweak it down the line.

General tips

  • Go through your changes after you’ve put them up for review, and mentally review your own code. Compare the styling to what we have elsewhere in the product and iterate on that until you’re happy and sure you’re matching us. That’ll save a lot of time on the review process.

UCOSP Fall 2013 Student Demos

Back in the Fall, we had a number of students hacking on Review Board.

At the end of the semester, when the grades are in, we usually invite our students to join us in a video chat and demo their projects to one another. It’s an optional thing that we do if the students have time, and not for credit.

Four students took us up on our offer last semester, and we’re pleased to share their work with you:

Review Board UCOSP Demos – Fall 2013 from Mike Conley on Vimeo.

The students in this video are (in order of appearance):

One student was unavailable for the demo session, but produced similarly awesome work:

Some of the projects in this video have already landed in the upcoming Review Board 2.0, and others are still waiting for some code massage before being available.

Thanks again to all of our students last semester!