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!

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 )

Google photo

You are commenting using your Google 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 )

Connecting to %s