Author Archives: rmdone

Meeting Minutes: Nov 23, 2014


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


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



  •  next weekend is technically our last meeting
  • So remember, you want to give us a few days to review your code
  •  so you’re trying to get your patches out of WIP and landed – aim to have that done a number of days – maybe 3 or 4 – before pencils down
  • to make sure we can get some review iterations in there
  •  I think I mentioned this before – your patches, despite getting ship-its, or getting into a landable state might not immediately land on master
  •  they might go into a project branch
  • don’t worry about that
  • ChipX86: important thing is that we get it into the codebase in some form
  • ChipX86: a lot of that is largely about release scheduling
  • after Pencil’s Down, our job is to produce an evaluation for your work this semester
  •  strictly speaking, after Pencil’s Down, the meetings are done
  •  however; we like to do this totally optional thing;
  • you’ve all been having your nose to the grindstone this semester
  •  and you each have been working on some pretty neat stuff
  • so we like to do demo sessions, where you can show off what you’ve done
  • they’re not long – 2 minutes or so, just showing how RB has improved with your changes
  • we also use that as an opportunity, after the demos, for you to give us feedback about the course
  • We like to know where we should improve for the next set of students – so if you have suggestions for us, or feedback, that’s a great time to deliver it
  • we like to hold the demos / post-mortem (that’s what we call the feedback session) one right after the other, and usually do it not long after the evaluations go in
  • and that way, you’re free to say whatever you’d like, with no risk to your evaluation (not that we’d shoot down your evaluation for giving us criticism)
  •  but still, we feel that gives you students some ease-of-mind when giving us feedback.
  •  Anyhow, I’ll send out mail over the next little while with a timeline
  •  I should stress that the demos / post-mortem are totally optional,  but they’re also totally cool.
  • Next week, I’ll likely launch into a talk about how to keep staying involved in the project even after the course ends (which is totally awesome to do and we encourage)
  • but I think I’ve taken up enough of this meeting time
  • have we mentioned that pencil’s down is 12/3?
  • andrewhong: will we be able to work on the ‘student’ projects after the term?
  • potentially, yes – though we might have other work for you to do as well if you’d like to stay involved. The possibilities are endless. 🙂
  • ChipX86: many of you are in a phase where you may be doing documentation work
  • ChipX86: Python doc strings, API docs, actual documentation files
  • ChipX86: and you may be wondering, how do I write good docs? Doc writing isn’t often something you think about when doing software development
  • ChipX86: this came up last night, and I’ll repeat the advice. If you’re stuck on the doc writing, and aren’t sure how to actually write it, pick up a rubber duck or find someone and explain what you’re doing to them. Explain it a few times. Then write that down. You’ll have a great start


  • m_conley: brennie: your django-evolution problem – do we have a forward path on that?
  • brennie: not really — i dont really know anything about how evolution works
  • ChipX86: brennie: was this the rel_field_name issue? or did something else come up?
  • brennie: yeah
  • ChipX86: django_evolution doesn’t need to be modified. RelationCounterField in djblets should just be updated to have rel_field_name default to None
  • m_conley: brennie: your other question was: “When it comes time to add support for rbtools should it require a flag (e.g. –with-history or -h or similar) to create the review? What should the flag be?”
  • brennie: yeah im not quit sure what a good name would be that wouldnt be too long to type;  –multi-commit-review-request is a bit long
  • m_conley: brennie: it can be easily changed later, so I wouldn’t sweat about it too much
  • brennie: I just want to make sure the api endpoint for /api/review-requests is reviewboard/webapi/resources/
  • m_conley: yep, should be


  • m_conley: dkus_: glad to see the backend land! congrats! at least, the form bits, anyhow
  • dkus_: ChipX86: I replied to some of your comments on
  • ChipX86: I’ll take a look
  • m_conley: dkus_: so, beyond reviews are you blocked on anything?
  • dkus_: Nope
  • m_conley: dkus_: ok, great – we’ll move along then. Thanks. 🙂


  • mloyzer_: m_conley, i just have my 2 questions
  • mloyzer_: as in the ones in the status report
  • m_conley: mloyzer_: I can work with you on the ActionHook one after the meeting if you’d like – at least, I can explain how ActionHooks work
  • mloyzer_: ok
  • m_conley: and that should help you figure out how ChipX86’s solution applies
  • mloyzer_: thats great thanks
  • m_conley: “Warning your working directory is not clean. Any changes which have not been commited to a branch will not be included in your review request.”
  • m_conley: hrm
  • m_conley: mloyzer_: when you say it doesn’t upload the correct diffs… what diffs *does* it upload?
  • mloyzer_: it uploads some docstring changeset
  • m_conley: mloyzer_: do you recognize that changeset?
  • mloyzer_: somethings i’ve never sen before
  • m_conley: like, is it one you’ve made?
  • mloyzer_: no
  • purple_cow: it sounds like something is funky with your git branches
  • m_conley: I agree
  • mloyzer_: so do you have any idea how to remedy it or?
  • m_conley: mloyzer_: can you stick around after the meeting? We’ll sort out your branches


  • nicole_xin: hi, i’m good i think
  • m_conley: nicole_xin: ok, no blockers?
  • nicole_xin: nope
  • m_conley: awesome, thanks
  • nicole_xin: thanks


  • m_conley: heapify / purple_cow / ChipX86: any thoughts on that first one?
  • m_conley: “How should I communicate back to the server when the slider is moved?”
  • purple_cow: for the first one, I think just loading a new URL is okay for now
  • rdone_: So all of that router logic would need to be added to the Text/Image view?
  • ChipX86: I would say, make it so that the slider itself is just emitting events when you change it
  • ChipX86: maybe the default is to reload the page with new query parameters or something
  • ChipX86: but a review UI’s javascript code should be able to just handle that, fetch the file attachments over the API, and re-render without a reload
  • purple_cow: I think given the time we have left, a reload is fine
  • rdone_: The event callback for a Review UI’s code just calls the router to adjust the URL, but I don’t think it does a complete reload
  • rdone_: Just the diff content, errr for the revuew UI’s slider
  • m_conley: Given the time left, what’s practical for rdone_ to get to?
  • ChipX86: rdone_: can you make the slider itself just send events, then have the review UI list and set up handlers that can be overridden, and have the handler just do the reload?
  • ChipX86: that’ll give us the flexibility we need
  • ChipX86: you won’t use the Router for this case
  • ChipX86: we’ll figure out how to integrate that down the road
  • rdone_: ok, so it will do a hard reload, in that case: I need to specify url parameters
  • m_conley: rdone_: does that clear up that question?
  • rdone_: yeah
  • m_conley: next question was:
  • m_conley: “How should image diffs be handled?”
  • m_conley: ChipX86: is this the onion-skinning thing?
  • ChipX86: yea
  • ChipX86: so the image diffs will just benefit from the reload currently
  • ChipX86: it was originally written for the diff viewer, btu we didn’t get all the pieces in. Works, though
  • rdone_: So I can use that for image diffs?
  • ChipX86: yep
  • m_conley: ok, last question:  “How might the user opt to turn off a file diff and just look at 1 instance of a file?”
  • rdone_: More of a UI design question
  • ChipX86: the diff viewer’s slider has a special value on the left: “orig
  • ChipX86: viewing a single revision means having the slider point to that and the revision
  • purple_cow: I think “orig” is a really weird thing to say for file attachments, thought
  • ChipX86: it is, but maybe the concept still works? I don’t know
    m_conley: let’s get it there and see
  • ChipX86: I don’t have any great ideas for that
  • rdone_: I figured you would never really need to do a single file view with an RR diff
  • purple_cow: maybe we can label it “no diff” instead
  • rdone_: but you might for a file attachment
  • ChipX86: right
  • ChipX86: btw, the slider shouldn’t appaer in the diff viewer’s view of a file attachment, since that’s controlled by the diff viewer’s slider
  • rdone_ I’m not sure I follow
  • ChipX86: the review UI for, say, images in the diff viewer (which is what image diffing was originally built for) should not have its own revision slider
  • ChipX86: I need to dig into your code more to see how it’s used
  • purple_cow: images in the diff viewer isn’t a publicly available feature yet
  • purple_cow: but a lot of the code is there
  • m_conley: rdone_: does that answer the question, or is this still unclear?
  • rdone_: The imageReviewableView is a child class of FileAttachmentReviewableView
  • rdone_ so wouldn’t that be ok?
  • ChipX86: that’s not the part that’s an issue. Basically, these diffable review UIs can be displayed now in two places:
  • ChipX86: 1) the diff viewer (currently requires some manual database editing)
  • ChipX86: 2) when viewing a file attachmetn on a review request
  • ChipX86: we only want the revision slider to appear on the review UI for #2
  • ChipX86 there’s a few easy ways we can tackle that
  • ChipX86: I’ll talk to you later about how to set up #1 to see it


  • m_conley: asalahli: you’re next
  • m_conley: asalahli: great job on getting rbt land out of WIP
  • m_conley: asalahli: what SCM were you thinking next?
  • asalahli: So, nothing much from me, except please review it as much as possible 🙂
  • m_conley: right, yep
  • asalahli: I was thinking of either svn or mercurial, but I realized I need to write a documentation as well. So I’m thinking thats more important, right?
  • m_conley asalahli: documentation would be grand, yes – but I don’t see that taking up the rest of the course
  • m_conley: so yes, please write the documentation
  • asalahli: agreed
  • m_conley: asalahli: but I agree, svn or hg support would be nice – at least, to get the ball rolling on it
  • purple_cow: in terms of demographics, I think svn is more popular than hg, but both are good
  • asalahli: I think hg will take less time since its very similar to svn
  • ChipX86: Mozilla may strongly encourage hg 😉
  • asalahli: but, I can try either one
  • m_conley: it’s true, we’re biased
  • ChipX86: I’d go with hg just because it’s a lot more similar to how git works than svn
  • m_conley: dooooo it
  • m_conley (there’s your bias right there)
  • ChipX86: svn might be really easy though, so it’s possible that if you have more time, you could quickly knock that out
  • m_conley: both would be incredible
  • asalahli: I’ll try to do both before pencils down 🙂
  • m_conley: asalahli: that’d be awesome
  • asalahli: thats it from me 🙂
  • m_conley: asalahli: but prioritize the documentation, then the other two
  • ChipX86: and of course, you’re more than welcome to stay on after the semester and tackle the rest 🙂


  • andrewhong: hello 🙂
  • m_conley: “I’m wondering how I can pull in file attributes like last modified, date created, etc. from externally (S3) stored files?”
  • m_conley: so, I found this:
  • m_conley: and I’ve kinda been flipping back and forth to it during this meeting
  • m_conley: trying to find a way to stat a stored file
  • ChipX86: so I can kind of answer this
  • ChipX86: or part of it
  • ChipX86: S3, while a file storage provider, doesn’t have a lot of filesystem concepts
  • ChipX86: there are timestamps, btu I think just one. There are no attributes in the sense that we care about
  • ChipX86: you can access the file’s storage through the File instanec’s storage attribute (iirc)
  • ChipX86: that has some functions like created_time() and modified_time()
  • andrewhong: hmm. i dont recall seeing that but ill dig at it again
  • andrewhong: there is a lot to look through heh
  • m_conley: yep
  • ChipX86: for attributes, just hard-code to 644
  • andrewhong: alright
  • m_conley: andrewhong: other questions?
  • andrewhong: are there any other attributes that are important?
  • ChipX86: I don’t think S3 has created_time(), just modified_time()
  • ChipX86: basically, you can’t assume a certain backend
  • ChipX86: so you’ll need to try/except these
  • ChipX86: they’ll raise a NotImplementedError if the backend doesn’t support them
  • andrewhong: right
  • ChipX86: I don’t think there are other attributes that matter
  • ChipX86: size, probably: (file.size)
  • andrewhong: ya ill take a second look later on
  • m_conley: andrewhong: anything else?
  • andrewhong: i could use some reviews whenever possible 🙂
  • m_conley: andrewhong: will get to those


  • m_conley: justy777: so you’re finishing off these hooks – that’s good
  • m_conley: justy777: you’ve got a few weeks left – I was combing through the issue tracker for things you could tack on towards the end
  • ChipX86: I had a thought for one thing
  • ChipX86: wouldn’t take up all the remaining time
  • ChipX86: m_conley: which did you find?
  • m_conley:
  • m_conley: justy777: thoughts on that?
  • ChipX86: that looks to have a lot of complicatedness around it involving Python compatibility, from the Python bug report
  • m_conley: hrm
  • ChipX86: well, actually, the last post on the Python bug sounds like it’s a solution
  • ChipX86: so sure, I’m for that
  • m_conley: justy777: sound ok?
  • justy777: sure, I’ll do my best
  • justy777: I’m also sandboxing those hook i just made
  • m_conley: justy777: that all sounds good – thanks for leaning into that stuff.
  • m_conley: justy777: any questions?
  • ChipX86: another thought on possibly tasks
  • m_conley: ChipX86: go ahead
  • ChipX86: justy777: we talked about the HostingServiceHook sandboxing, and I mentioned that several of those functions are allowed to throw exceptions
  • ChipX86: however, that’s super non-obvious without reading the code
  • justy777: Where is the code for reviewbot? and/or what are the settings on pep8?
  • ChipX86: maybe you could update the docstrings to indicate what exceptions are raised under what circumstances
  • ChipX86: we have customized pep8 settings in our install
  • m_conley: ReviewBot is here:
  • m_conley: justy777: other questions?
  • justy777: are those customized pep8 settings in a file I have access to?
  • m_conley: justy777: I believe the ReviewBot extension on the instance communicates those settings to ReviewBot
  • m_conley: hrm, I thought that was the case; heapify: –^?
  • ChipX86: it’s stored in our DB; we just pass a string to send along to the pep8 tool
  • ChipX86: what did you need them for?
  • ChipX86: actually, let’s finish the meeting and go into htat after 🙂
  • justy777: okay
  • m_conley: awesome, justy777: anything else?
  • heapify: (yeah, the PEP8 tool has some settings for which errors to ignore, etc.)
  • justy777: that is what i was looking for. None right now


  • ChipX86: next weekend we’re going to talk more about what to do if you want to stay on the project past the semester
  • ChipX86: for next semester, we’re going to be trying a replacement for IRC for the meetings
  • ChipX86: if any of you are already thinking of staying on, or even just want an easy avenue to keep in touch with fellow students/us, we’d like to invite you to the new replacment
  • m_conley: (It’s Slack btw
  • ChipX86: yeah
  • ChipX86: it’s like IRC but with a bunch of modern enhancements, easily accessible from mobile devices and desktop
  • ChipX86: so let us know
  • ChipX86: we have an #alumni channel in it
  • ChipX86: that’s it!

Meeting Minutes: Oct 19, 2014



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


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


m_conley (Mike Conley):

We’re pretty much here at the half-way point and we’re submitting your midterm evaluations in the next few days.
Now, I’m not entirely sure how UCOSP / your home faculty is going to distribute these midterm evaluations, but I did want to mention / remind you that whatever “grade” you receive as part of your midterm is what we (your mentors) think you would likely achieve at the end of the course if you were to maintain your current velocity.
This means that a not so great midterm evaluation does not guarantee a not so great final evaluation grade. Similarly, a fantastic midterm evaluation does not guarantee a fantastic final evaluation. It’s really just a signal to say either “Keep it up”, or “You need to pick up the pace.”
The final evaluation is what your home faculty will use to generate the grade that you receive from your particular institution – not the midterm.  At least, that’s what we (your mentors) have been told. It’s possible that your home faculty might do different things, which you can ask them about.
What I describe above is how things have been running with the universities so far.

Round Robin:

brennie (Barret Rennie)

  •  <m_conley> brennie: so I took a look at your diff comment expansion patch. You’re right – it’s pretty substantial. 🙂
  • <ChipX86> I plan on looking ati t soon
  • <m_conley> brennie: I know that purple_cow had some concerns with the UI – did that get sorted out?
  • <m_conley> I think I saw traffic in IRC about that… a conversation between brennie and purple_cow.
  • <brennie> m_conley: yeah. The controls are now hidden by default and appear when you mouseover, which solves the real estate problem. I also made it so the collapse button is a floating button like in the diffviewer, instead of a static button at the top/bottom of the diff.
  • <ChipX86> we were discussing stuff around I think the previous UI?
  • <ChipX86> brennie: you should link to the video you made in the status report
  • <m_conley> brennie: excellent, that sounds good. Oh! There’s a video! We love video. 🙂
  • <brennie> heres the first one
  • <brennie> and the second:
  • <m_conley> brennie: alright, excellent – well it sounds like you’re on the final phases of that patch, which is great. And all is well with rbt aliasing?
  • <brennie> I havent gotten a chance to start on it because fixing the floating comment button was not straightforward. I’ve decided that they should function as close as possible to git aliases (for consistency’s sake)

andrewhong (Andrew Hong)

  • <andrewhong_p> Trying to clean up my git repo up. Got a little sick over the past few days but healing up now… Did a few of those quick fixes as suggested on the review request.
  • <m_conley> andrewhong_p: cool. glad to hear you figured out the zip file corruption thing
  • <m_conley> andrewhong_p: what ended up being the issue there? I tried to piece it together from the diff, but it wasn’t immediately obvious
  • <andrewhong_p> Yes! I think it would’ve been easier to find if the package was better documented
  • <andrewhong_p> I wasn’t passing the stringio object to the zipstream constructor
  • <m_conley> andrewhong_p: the solution might make a fine blog post, and maybe file a bug against the package then?
  • <andrewhong_p> Definitely, a short and useful one
  • <m_conley> andrewhong_p: well, glad you figured it out. Yes, a blog post please, would be very lovely there, to help future folk.
  • <m_conley> andrewhong_p: and so you’re looking for more work now?
  • <andrewhong_p> Yep I’ll be looking at student projects again next week
  • <ChipX86> let us know if nothing on the list is interesting. There are lots of things on the roadmap
  • <m_conley> andrewhong_p: and yes, please either select or alert us quickly – not having work to do is not a great state to be in.
  • <andrewhong_p> Sure, i picked out a few already

dkus_ (David Kus)

  • <m_conley> dkus_ wrote – “I noticed that David T. (purple_cow) added some feature around file attachment history. How should user file attachments handle this? As it is right now, my code ignores this feature.”
  • <ChipX86> you can ignore that; <m_conley> I think ignoring it is the right idea
  • <dkus_> ok
  • <ChipX86> that’s a feature designed to allow you to replace file attachmetns on a review request; like diffs
  • <m_conley> and see the history of those attachments over time
  • <m_conley> (actually the result of a student project – lyubinets’s, to be exact)
  • <m_conley> dkus_ also writes: “What should happen when using the view to try and view a file attachment before the file has been uploaded? e.g. View at /attachments/user-files// . This should redirect to the actual file for FileAttachment object with id (e.g. /uploads/etc/etc/). If there is no file associated with this FileAttachment object yet, what should it do?”
  • <purple_cow> I would think 404?
  • <m_conley> sounds like the webby thing to do
  • <ChipX86> yeah, probably 404
  • <dkus_> ok
  • <m_conley> dkus_: other questions or issues?
  • <dkus_> does that url make sense for the attachments?
  • <dkus_> /attachments/user-files
  • <purple_cow> that should be fine
  • <m_conley> dkus_: cool – all set to work on the front-end bits? Seems like the back-end is pretty much there.
  • <dkus_> yeah, thought i would try and add a few tests around the web api if i could
  • <m_conley> dkus_: that’s a great idea; dkus_: there should already be some attachment tests lying around that you can use for inspiration
  • <dkus_> yeah I was going through them earlier
  • <ChipX86> yep, definitely add some tests; everyone should strive to write tests for any code they write
  • <ChipX86> it’s the only way we know for sure that your feature won’t break down the road when something changes
  • <m_conley> they might suck to write, but maaaaaan can they save your butt in the future; it’s basically a seat belt, but for your code.
  • <ChipX86> some of the most important code you’ll write are tests

nicole_xin_ (Nicole Xin)

  • <m_conley> nicole_xin_: looks like you’re out of WIP for the back-end stuff. Great job!
  • <nicole_xin_> i’m done with backend models!; and i passed all the tests i wrote
  • <m_conley> great – we’ll have some reviews on that soon, but don’t be blocked by us (or if you are blocked by us, please alert us ASAP)
  • <nicole_xin_> i’m almost done on web api, just one new test to add
  • <m_conley> ah, tests. Music to our ears.
  • <nicole_xin_> yes sure
  • <m_conley> nicole_xin_: you said something about a database crash in your status report
    <nicole_xin_> yes; but i have no idea why; just got a new one solves everything
  • <m_conley> I just wanted to remind everybody that their development environments (unless you’re doing something freaky) are using SQLite as the database backend, and your database is a single file, called reviewboard.db, and it’s in your root directory
  • <m_conley> and you can copy and delete that file as you please
  • <ChipX86> and you should do that fairly often
  • <m_conley> I recommend backing it up just before doing anything interesting with the database – like running syncdb or an evolve
  • <nicole_xin_> great tips
  • <ChipX86> protip: append timestamps to the filenames when you back up
  • <nicole_xin_> i did spend a lot of time fixing it, just didn’t work
  • <ChipX86> if there’s something interesting about them, append to the filename (e.g., pre-evolve-for-branchname)
  • <m_conley> even pro-er tip, have a cronjob make the backups and filenames for you, maybe daily or every other day, or weekly.

rdone (Ryan Done)

  • <m_conley> I was in the middle of reviewing your screenshots when the meeting started – but I guess I can tell you here: I wonder if we might somehow connect the selected letter with the page numbers a little more strongly…
  • <rdone> Any suggestions on how to do that?
  • <m_conley> like, surround them with the same background colour, to make them seem like the same unit
  • <m_conley> So: suppose we make the selected letter in general stand out more, by not just bolding the letter, but giving it a background colour for more contrast, or at least, to draw the eye more
  • <rdone> Ok
  • <m_conley> since it’s easy to get lost in that sea of letters, and not immediately see which one is selected; colour can draw the eye
  • <rdone> Just the letter? Or the letter and number controls?
  • <m_conley> well, if there are number controls for a letter, yes – perhaps we can experiment with sharing the same background colour
  • <m_conley> also, for your spam fighting extension, had anybody talked to you at all about how we would identify spam?
  • <purple_cow> I think the idea was that new users would be moderated for a time
  • <m_conley> purple_cow: oh, so this was more of an approval process, then something like Akismet?
  • <purple_cow> since the issue isn’t so much traditional “spam” as it is people who are just playing around
  • <ChipX86> some things that could be considered as well are reviews made on closed review requests by new users, like, red flag that
  • <m_conley> we should come up with a list of heuristics
  • <rdone> The project description sounded like it was catching newly registered accounts and giving them limited privleges until approval
  • <rdone> or should it be more retroactive: catching ‘spammy’ behaviour
  • <m_conley> rdone: in any event, the end result will be that some reviews / comments / replies are going to need to be mapped to some state of either hidden or not hidden.
  • <ChipX86> one thing I’d also like is a sort of Spam button that, with a prompt, lets me delete the review and optionally ban the user on a review
  • <purple_cow> personally, I’d say that catching newly registered accounts is simpler and it’s a rare enough problem that it wouldn’t be too onerous
  • <ChipX86> sure, but we do have a lot of accounts in our database right now
  • <purple_cow> ChipX86: we do, but we only get new users posting stuff maybe once a week
  • <ChipX86> it would also be super cool for projects like ours and apache to be able to look for registered spam accounts nad delete them
  • <ChipX86> I think there are blacklists to compare against somewhere out there
  • <ChipX86> btu anyway, jsut food for thought
  • <m_conley> rdone: are you able to stick around after the meeting to solidify this stuff?
  • <rdone> for sure it seems like theres a bunch of different requirements here
  • <m_conley> rdone: yeah, I think we need to figure out the scope of this thing

mloyzer (Mark Loyzer)

  • <m_conley> mloyzer: hey – so, I like your PDF formatting, and I think we should go with that
  • <mloyzer> m_conley, ya, i saw your review
  • <m_conley> purple_cow / ChipX86: not sure you saw from our last meeting minutes, but mloyzer, smacleod and I were pretty certain that CSV doesn’t make a whole lot of sense for review request export, due to the one-to-many relationships with various things (bugs, reviewers, reviews, etc)
  • <m_conley> so we thought instead of doing a CSV export, mloyzer could do XML instead.
  • <mloyzer> m_conley, i started with the more fundamental (harder to screw up) information anyway so I’ve done a fair amount of translating the review_request int a pdf
  • <m_conley> cool
  • <ChipX86> I have no problem with an XML export. I did wonder about CSV in that regard
  • <ChipX86> people can always take an XML dump and XSLT it to some preferred CSV format
  • <m_conley> enterprises love XML; so, we’re a GO on XML
  • <mloyzer> m_conley, ok cool. I want to finish PDF first anyway now
  • <m_conley> mloyzer: “What is the difference between a review’s body_top, body_bottom, and comments? In order to add a ‘body_top’ I had to click the ‘add comment’ link which I assumed would generate and save the contents as a comment…but it was apparently the review’s body_top. So, in short, what is the difference between these?”
  • <mloyzer> m_conley, yup^. was just confused/curious about that
  • <m_conley> so, a review usually has comments on things like diffs or attachments, but not always. Like, sometimes there are general things to say that are not related to a particular line in a diff or a file
  • <m_conley> if you were to open a review on somebody’s review request right now, like, click on the “Review” button in an open review request, without having commented on anything
  • <m_conley> the dialog that comes up has a text input for your review. That’s the body_top.
  • <m_conley> It’s associated with a review, but not to a particular diff / line in a diff / screenshot / file.
  • <m_conley> mloyzer: now, if you were to comment on the diff, for example, when you went to edit your review
  • <m_conley> you’d see this:
  • <m_conley> that Add Header link exposes the same text input that you would use before if you hadn’t have commented on anything
  • <m_conley> and Add footer does something similar, but as kind of an “afterward” for all of your comments.
    <m_conley> so, in sum:  A review can have a body_top / body_bottom, which is text associated with a review request, but not associated with a particular diff / screenshot / file. The body_bottom ability is only exposed if comments on diffs / screenshots / files already exist.
  • <m_conley> as an ‘afterward’
  • <mloyzer> ah ok.. makes more sense now anyway 🙂
  • <mloyzer> i’ll have to look into it more when I start transfering the information from reviews to PDF but that does help

justy777 (Justin Maillet)

  • <m_conley> justy777: do you have any questions to help you move forward?
  • <justy777> just a small question
  • <justy777> once i have tests and the actually sandboxing, should the test and sandboxing go onto the same review request for that extension?
  • <ChipX86> yep
  • <justy777> okay, that’s all i got for questions
  • <m_conley> justy777: I notice in your status report that it says you weren’t able to do much research this week – though that implies you did some. I would highly recommend you post those research notes in a Hackpad somewhere, so we can see them.
  • <m_conley> justy777: best if you could put those up immediately after this meeting.

salahli (Azad Salahli)

  • <m_conley> asalahli: so, your question is about whether or not we’re duplicating code by having rbt land do essentially what rbt patch does, but then doing a push?
  • <asalahli> yeah
  • <m_conley> asalahli: I think we should totally leverage rbt patch if we can.
  • <asalahli> thats what I thought
  • <m_conley> asalahli: I thought we discussed this with smacleod a few weeks back though – I’m pretty sure I brought up the fact that rbt push does much of what rbt patch already does…
  • <ChipX86> something that would also be good is to automatically add the “Reviewed at” on the commit message
  • <asalahli> rbt push?
  • <m_conley> asalahli: do you remember the result of our discussion with smacleod along those lines?
  • <purple_cow> ChipX86: well, we do that already in ‘rbt patch’
  • <asalahli> m_conley: yes, you said I should look at rbt patch, because it is very similar
  • <ChipX86> so, yes, but if I’m pushing my own change, Id on’t want to use rbt patch
  • <ChipX86> that’s a place where this differs
  • <asalahli> but it happened to be more similar than I expected; thats why I wanted to confirm
    <ChipX86> I’d think rbt land would be more of a wrapper around rbt patch, the SCM’s merge/rebase and push
  • <ChipX86> I don’t know what the previous discussion is, btu I definitely have thoughts on this command based on talking to another company that wrote a similar tool
  • <ChipX86> so maybe we should sit down and discuss it more?
  • <m_conley> sounds good
  • <asalahli> ChipX86: That’d be awesome
  • <m_conley> asalahli: can you stick around after the meeting?
  • <m_conley> let’s get this thing ironed out
  • <asalahli> m_conley: yeah
  • <asalahli> mentors: Can I haz more reviews to my RR please?
  • <m_conley> heh, yes, you can haz. Sorry, we’ve all been a bit backlogged with a few other things.
  • <asalahli>
  • <m_conley> ^– other students – this is a great time to do some reviews too…
  • <asalahli> dkus_: I added students group to the RR
  • <m_conley> asalahli: yes, we’ll get in there. Thanks for hounding us. 🙂
  • <asalahli> how do I build documentation locally?
  • <m_conley> ChipX86 / purple_cow / heapify: is that a command?
  • <asalahli> I still haven’t fixed WEB API docs, but I think I know how to do it
  • <ChipX86> go into docs/manual/; run: make html
  • <ChipX86> you will need sphinx and maybe a couple other things installed
  • <ChipX86> (make html will build docs, but they will look different than what’s on the website)

Status Reports: Sept. 28, 2014


What project are you working on?
What you accomplished this week
Links to anything you’ve done this week
What you plan to do next week.
What, if anything, is blocking you from making progress?
Any other questions

Ryan Done:

What project are you working on?
Currently working on updating the User List Page paginator to allow querying by name starting letter.
What you accomplished this week
Submitted fix for a bug in the unit tests where certain unit tests for CVS were being called and failing if the machine did not have CVS installed
Fixed and submitted my easyfix bug: GET queries for all review requests to the web api as admin were not returning private (unpublished, ‘public’ in the review request model) review requests.
Picked out my first project: Adding some features to the users page and djblets datagrid.
I got a first implementation of the alphabetic paginator for the users list and submitted my first WIP reviews for them (Djblets and Reviewboard) when I got home.
Did 3~4 code reviews.
And of course I’m collecting the status reports this week.
Links to anything you’ve done this week
What you plan to do next week.
– Make a hackpad and update with some details of what I’ve done and will do for my current project
– Finish the alphabetic paginator, at least for the users list. Theres probably about a days more work testing it and making making some adjustments, then of course addressing any review suggestions etc.
– Determine if any other datagrids in reviewboard could use it and implement it there. (Open ended, but ideally I will do the users-page in such a way that its a matter of just enabling the alphabet paginator on other grids).
– Start on the live search box for the datagrid (part of the project)
(Open ended, will carry onto next week)
What, if anything, is blocking you from making progress?
When I got home I had some issues migrating my code repo back to my desktop (Windows Machine). Git was saying that every file everywhere was modified and I couldn’t perform any useful operations. Long story short the problem is to do with line ending settings. ( I lost about a day working that out but I’m all good now.
Any other questions
All good for now.

Andrew Hong

What project are you working on?
I am currently looking into allowing users to download a zip of all file attachments that are apart of a Review Request.
What you accomplished this week
– Issue 3444: autocomplete is too agressive — tackled during the sprint, thanks for all feedback and comments, it is now shipped!
– Read through all the student projects to see which I’m potentially interested in, went with ‘downloading all file attachments from Review Request as zip’
– Performed a few code reviews, maybe 3-4?
– Currently looking into different ways of approaching the current project, I believe I have found all the files that will need modification to enable this feature.
Links to anything you’ve done this week
Issue 3444: Autocomplete is too agressive:
Project: Download All Button for File Attachments
What you plan to do next week.
Hopefully get the main chunk of this feature working.
What, if anything, is blocking you from making progress?
Nothing, all is good!
Any other questions

Not at the moment.

David Kus

What project are you working on?
Currently working on “Drag ‘n Drop Inline Images”. Adding the ability to drag and drop images into the markdown editor (when commenting, adding review description, etc.) instead of having to upload the images first and then link to them.
What you accomplished this week?
Issue 3437: Double-clicking to add a comment in the diff view discards any comments made.
  – Worked on this during the code sprint. Made it so that when clicking to add a comment, if there is already a comment dialog open for the same region that you selected, it doesn’t open a new one. Status: Shipped.
Project “Drag ‘n Drop Inline Images”:
  – Took a look at the prototype created by a previous student for the project. Got that working on my dev instance. Did some work on overriding the whole “drag ‘n drop” overlay for file attachments on review requests, so that when you want to drag and drop an image into a text editor it doesn’t get blocked by the overlay.
Links to anything you’ve done this week.
Issue 3437:
Review Request for Work in Progress:
HackPad (kind of empty, but i’ll be adding notes here):
What you plan to do next week.
Keep working on the project. Start looking at the back-end more. The prototype that was created for the back-end seems to work pretty well, going to test it some more and try to clean it up. Also going to work harder at keeping notes in hackpad. Going to add some notes on the stuff that I did last week as well.
What, if anything, is blocking you from making progress?
Nothing at the moment

Nicole Xin

What project are you working on?
I’m currently working on “General comments(with issues)”. Reviewboard now support comments on diffs,file attachments and screenshots, but there is no generic comment type that can open an issue tracking. So we would like to add that.
What you accomplished this week?
1. Fixed my Easyfix bugs(Issue 3532: linking to a collapsed review does not expand the review) apparently. It was fixed by parsing the url and force the linked box to open, and it was shipped during the code sprint.
2. Did a few code reviews.
3. Picked the project “General comments with issues” and started thinking about what tasks to do and where are the file located and took a few notes.
Links to anything you’ve done this week:
(Lots of issue opened by Review Bot. I have to leave shortly so I’ll fix them later.)
What you plan to do next week.
– Update my hackpad with any new ideas and hopefully fill in more details.
– Get more familiar with the dependencies and models.
– Keep working on my project, hopefully the back-end model can be completely set up next week.
– Learning Django
What, if anything, is blocking you from making progress?
Nothing yet. Haven’t been working too much after the code sprint, will definitely make a difference next week.
Any other questions
Nope, everything is fine.

Justin Maillet

What project are you working on?
I’m working on the “Sandboxing Extensions” project.
What you accomplished this week
I got my easy bug fix submitted, reviewed, and committed.
 My Hackpad has been created and updated.
Finally, I’ve been writing tests for the sandboxing project.
Links to anything you’ve done this week
What you plan to do next week?
Next week, I plan to learn even more python, and get about three pieces of the sandboxing project done.
What, if anything, is blocking you from making progress?
Nothing is blocking my progress.
Any other questions?
I’ve got a few questions about the Google Summer of Code, but I can ask those after the meeting.

Mark Loyzer

What project are you working on?
I am currently working on adding an extension to Review Board that will allow people to export a Review as PDF or CSV.
What you accomplished this week
Fixed and submitted my easyfix bug: Prevent users from publishing empty (no modifications made) review request drafts (@
Picked out my first project: “Export review data”
I had to do some reading on extending Review Board at and ran into some problems with the documentation but figured it out through trial and error.
Did 3 code reviews.
Links to anything you’ve done this week
I haven’t submitted a WIP yet because it was mostly fixing some minor things with boiler plate code.  I will definitely have a WIP for next week.
What you plan to do next week.
– Keep my Hackpad up to date (I forgot to at the Sprint so I had to go back and try to recollect everything I had done and such).
– Figure out how request/response works with extensions so I can then start looking into how to aggregate all the data associated with a review.
What, if anything, is blocking you from making progress?
I was stuck with getting the extension up and running for a couple of reasons but the biggest one was I didn’t know how to ‘enable’ the extension.  I thought that once you installed it (ran the file) it would automatically be loaded into the application.  I figured out that you needed to login as an admin and enable the extension through the Administration UI.
Any other questions
Not at the moment.

Azad Salahli

What project are you working on?
I’m working on “Update ‘rbt post’ to use the “Validate Diff” API resource”. My estimation of project completion is 2-3 weeks.
What you accomplished this week
I have fixed EasyFix bug number 3438 ( I also did a minor change to Getting Started guide, and did a minor refactoring in RBTools codebase.
Links to anything you’ve done this week
What you plan to do next week.
My goal this week is to work on ReviewBoard API to create new flag to avoid validating the diff on upload if it is already validated,
What, if anything, is blocking you from making progress?
Nothing as of now.
Any other questions
No question so far.

My goal this week is to work on ReviewBoard API to create new flag to avoid validating the diff on upload if it is already validated,

I wouldn’t spend any time on this. Either way we’ll have to re-parse the diff, and the file existence state is cached. No matter what, it’s best to get things working correctly before thinking much about performance–if it turns out that it feels slow, then we can investigate how to make it faster.

– Reply from David Trowbridge

Barret Rennie

Currently I just finished exclude pattern support for perforce. This was the last SCMClient (besides ClearCase and Plastic) that did not support it. I will be moving on to adding incremental diff support in code reviews.

Since the last status report (including the code sprint), I’ve done: (-X flag) (hg -X support) (git -X support) (svn -X support) (fix git -X bug) (refactor svn’s _filter_diff) (bzr -X support) (p4 -X support)

In the upcoming week I plan to first get the UI addition for the incremental diff addition, then dig into the RB api to see if I’ll have to add new API features to accomplish this.