Meeting Minutes for December 2, 2012


  • yangtina was absent from the meeting due to an exam, but will be present at the post-mortem.
  • This is the last official meeting of the term.
  • Post-mortem will be held on the Sunday after next (December 16).
  • This Tuesday (December 4th) is firm pencils down at 11:59PM PST.

Questions / Answers / Tips:

Q (slchen): Regarding your comment here: regarding possibly adding XML thumbnails as part of the XML Review UI Extension – I’ll have to do some digging and add a new hook in reviewbuard/extensions/, but I think it’s doable. How should I proritize that?

A (purple_cow): Low.

A (slchen): Here are the possible tasks on my plate: first and foremost is getting the commenting functionality done for TextBased Review UIs. I have been working on that with aam1r since yesterday, and he’s doing most of the backbracing on it since I’m much less familiar with JS. I think we have most of it done, but aam1r’s mostly working solo on it now and I’m just helping him trouble shoot when he runs into a block

Q (m_conley): does it look like something you’ll be able to drop-in and use for XML?

A (slchen): it is, we are trying to generalize it for Text-Based Review UIs in general

Q (slchen): and the other thing I’m looking into is documenting other hooks; so my other question is are there anything else for me to do in case I get blocked on any of the current tasks I have?

A (m_conley): considering the closeness of pencils down, you might already have enough

Q (tahnok): so buildbot can apply patches to a branch/revision other than master and reviewboard has a branch field, should I be getting the branch field in the reviewrequest? or does reviewboard store branch information internally?

A (smacleod): As ChipX86 said, that branch field really is just freeform text

A (ChipX86): you can’t trust it

A (purple_cow): we should maybe look into adding a hidden field for the base revision of a change, which post-review could fill out

Q (tahnok): but patches can applied to anywhere. should I add an option to use that field?

A (ChipX86): we have a base revision, the source revision of a file

A (purple_cow): but that’s per-file, and to build, we need the whole tree

Q (ChipX86): can we not figure out the commit from that?

A (purple_cow): not that I know of

A (tahnok): at least that way users would know that if they need to use a different branch they can, and there’s no reason why the patch has to be applied against the base-rev. it might make more sense to have it applied to HEAD. I think that by default it uses master, but there’s an option to use the branch field

A (purple_cow): let’s start with master for now

A (ChipX86): what would be kind of cool is to say “Here’s a list of branches, make sure it works on all”

Q (tahnok): could the list of branches be global for the plugin? there could be an option field that takes a comma seperated list of branches?

A (smacleod): Yeah, I think that works

A (tahnok): I’m just worried that you could have reviews for different projects on the same installation, and that they wouldn’t all have same branches.

Q (smacleod): hmm, sorry to throw another wrench in this, but Review Bot isn’t repo intelligent at the moment. Doesn’t discriminate between repos 😛

Q (ChipX86): we could attempt to look up by the field and hope. then if an install uses buildbot, they have incentive to make sure it’s correct. if it’s blank, assume master. it’s just that it is freeform. when DVCS stuff starts to happen, there will be a better concept of branches

A (aam1r): no questions. as slchen mentioned, we have been making good progress on the comments. we got it working for markdown. we are now generalizing it so it works for any text-based review UI. should be up for a review request today.

Q (Allyshia): So I have updates to my latest review comments in the works — just looking for some indication (based on conversations with some of the mentors, status reports) of what seems to be incomplete that should really go in before Tuesday (by the end of my current changes, JSLint should be “complete”, manual trigger will still be in progress, but my portion should be complete … unless there was anything else that someone thought of that should get in right now).

Q (m_conley): what’s left to do with manual trigger?

A (Allyshia): well the UI will be done, but the actual trigger functionality won’t be complete, neither will permissions

Q (m_conley): the licensing thing got all sorted out?

A (Allyshia): with JSLint, yes.

Q (Allyshia): I’m having this frustrating problem regarding updates to ReviewBot (I get the feeling my local repo is out of sync but won’t sync) but I think I can troubleshoot that with smacleod afterwards if he’s OK with it

A (smacleod): yeah, we’ll fix it for you

Q (johns187): I’m going to refactor my code, write ‘rb publish’ — what other commands can I do?

A (purple_cow): at this point I’d really like to see some effort on removing the unnecessary options instead of other commands

A (jzamb): I don’t have any questions at the moment. Just want to say i just posted for review and it’d be nice to get some feedback.

A (Karl-L): No questions, but still need more review for r/3488. It’s been through a few rounds, and the issues raised up until now have been dealt with.

Q (Michee): I still haven’t figure out how to view localhost outside of virtualbox, I don’t know if anyone can help me. So as of this point, I’ve only tested my image diff page on Firefox in my (extremely slow) ubuntu inside virtualbox.

A (m_conley): I might be able to, after the meeting. I think for VirtualBox, there’s a portmapping thing you’ve gotta do.

Q (Michee): That’s also something I’m a little concerned about. My OS runs really slowly so I don’t know if that might make me see things differently.

A (m_conley): the rendering should give the same result, regardless of the speed. the interaction might just be sticky. :/

A (Michee): I just pulled my code out of WIP so a code review would be muchly appreciated.


m_conley: I just wanted to say that this has been a fantastic term. I’m really pleased to see how we’re getting students reviewing other students, etc. big thumbs up for the folks who’ve been doing that. I also wanted to remind you all that no matter what, when this course is over and done with, you’re still welcome to hack with us on Review Board. we’re going to have a new crop of students next semester, and they’re going to need help and reviewers. and there’s always plenty of work. and we like you folks. 🙂

ChipX86: (the work! It won’t end!)

m_conley: So please, stick around if you’re having fun. Yes, we’ll always have work to dish out. 😀 Great work everybody.

ChipX86: if you liked what you did, if you want to in a sense “own” some of that work going forward, we’d be very happy to have oyu on board

m_conley: the more the merrier, really. it always makes me happy to see UCOSP names in the release notes

ChipX86: and if any of you are ever down in the Bay Area/Silicon Valley, hit us up 🙂

m_conley: ditto wrt Toronto. I lunch.

smacleod: (I’m in cincinnati, so if for some crazy reason you end up in OH, let me know… probably not likely tho haha)

ChipX86: and feel free to remain here in IRC. anyone who’s planning to use [the bouncers] to be at least somewhat active in here, stay signed on and I’ll keep them going. otherwise, quit so I can see later what to prune.

(tl;dr: Continue to hack on Review Board if anyone is interested. Hit up ChipX86 in the Bay Area/Silicon Valley, m_conley in Toronto, smacleod in Cincinnati. Stay on IRC if you want to keep your bouncer.)


Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s