Meeting Minutes: July 20, 2014

Attendees

Mentors: Christian, David, Anselina, Mike, Steven

Students: Volodymyr, Salam, Raheman, Matthew, Peter

Announcements

  • Please review each other’s code this week, even if it’s just to test out patches (rbt patch will be useful here)! You can look at style, architecture, edge cases, whether variable names are clear, etc. Are there steps that don’t make sense or can be combined?
  • There’s one month left! Take a good look at the calendar and at how much work you have remaining – if it’s starting to look like you’re not going to hit a target, please let us know ASAP, and we’ll work something out.
  • Make sure to leave a few days at the end of your project for review iteration – having your patches out of WIP at least 1 week before pencils down is a good idea.
  • What you have could already be landed as-in – it doesn’t have to land in one big chunk.

Round Robin

Volodymyr

  • Pretty sure that the Backbone events syntax doesn’t assume the element you’re targeting is the direct descendant of the view element?

    It actually does. It’s listening on this.$el, catching anything that bubbles, and comparing the targets to that list. If you need anything more specialized, you have to listen to it yourself. Assigning it yourself is not too hacky.

  • For binary files reviewing, modified the Python side so if no review UI was found the file, the generic review UI will be used. Not sure what’s best to put on that page – just title, download link, and a revision slider?

    Just a filename, caption, and download button is fine. It’s duplicating the thumbnail a bit, but the slider’s providing additional value, and it’s still a thing that can be linked to. Could be nice to present the comments made to that attachment and allow for creating a new comment, but this is extra work. Come up with a quick mockup!

  • Would appreciate another round of reviews.

Salam

  • Got some WebAPI stuff up!
  • Should work on the settings page (template management UI) next or test the API?

    Functionality gets higher priority over automated tests, so work on the settings page.

  • How should I get started?

    What you’ll probably want to do is create a views.py, and register a URL to point at it. A URLHook can be used to point at some function in a views.py in your extension directory. After this, you’ll have your own page registered.

Raheman

  • Having trouble figuring out where to put some initialization logic for a model attribute. Need to modify some fields (like file_usage, file_quota) in the user owning the file attachment, whenever the file attachment’s user field is modified. Having problems accomplishing this whenever a model instance is created.

    We don’t really need quota support right now, so just allow unlimited file attachments per user. Users would never be able to delete their stuff since it’s a part of the history (which we never delete). A maximum per-file size would be useful as a global setting in siteconfig.

    To answer the original question, the best solution would have probably been to override FileAttachment.save(), and do the logic in there. It can check if it’s new or not by checking if self.pk is None.

Matthew

  • Which version of Review Bot and Review Board should I be on?

    Generally, master for Review Board since release-2.0.x is going to be largely just bug fixes. Review Bot’s master is broken for now, so work on release-0.2.x.

  • Have some general questions on Hackpad for smacleod to answer.

Peter

  • Styling patch for TogetherJS should be landed soon!
  • Successfully set up an independent hub for TogetherJS and tested it with the local hub.
  • Need to bundle the JS instead of using templating, which should be quick.
  • Moving onto scroll detection. Planned high-level steps:
    1) Print current scroll position.
    2) Send position to peer.
    3) Animate the new position.
    Any intermediate steps to focus on that could help?

    Having two machines connected, scrolling in one, and having the other side hear it is a huge step. Just make sure that the signalling even works first. Pretty sure TogetherJS does the job of sending scroll positions to everybody else already, but it’s up to the implementor to listen for these messages.

    There’s also a data channel you can send anything you want over, so even if it doesn’t send scroll positions by default, we could still send it ourselves.

  • Any demos of how to animate the scroll position?

    Could have indicators for each user, and let people click to jump to the same scroll (kind of like Hackpad). Could also have an optional sticky scroll feature where one user just follows another user’s position, for guided code reviews.

    As a first step, just display a horizontal line, say, 25px wide, to represent a user’s scroll position.

Advertisements

One thought on “Meeting Minutes: July 20, 2014

  1. Pingback: Meeting Minutes: September 7, 2014 | Review Board Student Blog

Leave a Reply

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

WordPress.com Logo

You are commenting using your WordPress.com 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