Meeting Minutes: September 14, 2014

Attendees

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)

Announcements

  1. We (mentors) won’t always get to review requests immediately or even notice them, so if it’s been a day since you posted, point us to it directly
  2. Fill out your description and testing done thoroughly when doing RR. We need to know exactly what the change does and how you tested it in order to know what to focus on, and this information will also go into the commit message. This is also a good read: https://www.reviewboard.org/docs/codebase/dev/git/clean-commits/
  3. If you got a Fusion 6 license, and wanted Fusion 7, let ChipX86 know!

About the Sprint (m_conley):

I think there’s a start time of something like 9AM or so.

There’s no attendance or anything, but getting there on time means you’ll hear any starting announcements. Also, there might be donuts – Karen is known for bringing in donuts – and if you want a donut, well, you’d better hustle.

End times are around 5-ish, 5:30-ish? It’s really kinda fluid, and I think it depends on how dinner gets organized. I believe there’s a night where dinner is taken care of? I don’t know that for certain. But if so, that’ll likely involve a large reservation, so the timing will be organized more strictly. But 5-ish is about when the day ends.

I’m hedging because I’m going off past experience, and not from an official schedule or anything.

Even when there isn’t an organized dinner, we usually go out as a team. It’s a good idea, though not compulsory, to stick around in the evening and hang out, like, to get to know your fellow students, put names to faces in more concrete, for-reals-socially ways. I encourage you to participate – we’re all great people, I can tell.

If your inner introvert is freaking out, don’t worry about it – you’re surrounded by likeminded, interesting people who know how to speak your language. My inner introvert freaks out just a bit every time too.

ChipX86: when you get there, have m_conley tell you the horse head story

m_conley: I won’t be in town for most of the sprint, but if I have the strength after returning on Saturday night, I’ll show up for Sunday, and then tell the horse head story

Round Robin

brennie

  • I’ve got no questions
  • Waiting on a review for my review request (https://reviews.reviewboard.org/r/6317/)
    • m_conley: This is also an excellent time for you all to get some reviewing under your belts. I know what you’re thinking – “I’m new at this, how can I possibly review this code?”. Just take a look at the diff and see what you think. Maybe something is not clear, or maybe something looks really excellent, or something could be done more efficiently, etc. If so, comment on it. There are some tips for reviewing code in your welcome packet
  • Apparently ClearCase and Perforce are only paid for commercial use so I should be able to test them myself.
  • Q: do you know what you’re doing this week then?
    A: Yep. First off -X for hg (which should be easy because hg supports -X) then git and try the other SCMs

rdone

  • I am no longer working on issue 3452, mloyzer has taken over that, and I switched over to 2766
  • For the week I’ll just be working on that
  • In working on that found another bug, filed an issue ticket, and then ChipX86 resolved it
    • ChipX86: we already had it up for review by the time that was filed

dkus

  • Nothing outside the status report.
  • Saw that Chipx86‘s reply to my question. Haven’t had a chance to look over it yet, but this week i’ll be working on issue 3437

nicole_xin

  • I do have some questions with bug 3532, which is what I’m working on.
  • There is a feature that when we have a link to a comment, the corresponding review box will be expanded. Somehow I have no clue where this part locates.
    • m_conley: That’ll almost certainly be in the static/rb/js
    • ChipX86: It seems the browser is actually handling the visibility of that box, and that maybe it doesn’t work for reviews because the <a> tag for that anchor is outside the box. So one thing to try is seeing what happens if that’s moved inside somewhere. If so, there’s a couple ways to fix this. Either move it inside, or we have some code that checks the anchor, matches it to a review box, and then opens it. Actually, the view function in Python (review_detail) computes whether boxes are open or closed by default, so that could actually match up the anchor and just force it open. If you have that code work for both reviews and comments, then it’ll fix the opening and the initial button state in one go
  • How to know a review id or comment id?
    • ChipX86: You’d have to parse the URL (there are Python functions for this) to see the anchor being used. We have a standard naming convention (not a good one, but a standard one) where we have prefixes of “review”, “comment”, “fcomment”, or “scomment”, along with the ID of the object.
  • I mean, how do I test this? To find my own review id
    • ChipX86: You can open the browser’s developer tools and inspect the review box and look for the nearest <a name=”review12345″>. Most of you are going to spend a lot of time in the browser’s developer tools. Good to get comfortable with it

justy777

  • I’ve been looking at issue 3300, and trying to get a feel for how things are organized.
  • Q: Do you have your hackpad set up?
    A: No, I will be doing that today after the meeting.
  • Q: You know how to move forward?
    A: Yes keep working on my bug in static/rb/js

andrewhong

  • Laptop been having issues since I went to Windows 8. Hoping everything is well once I go back to Windows 7. Setting it up now.. Shouldn’t take long to get back.
    • Mentors: Machine troubles are one of several excellent reasons why we’re going to be asking for WIP code drops every week. If you want to be extra careful, store everything in Dropbox or a Github fork of Review Board or RBTools, Djblets, whatever it is you’re hacking on.
  • In terms of RB: going to try and make more progress on the autocomplete bug (3444) this week. Still picking up Django as I go

asalahli

  • I still cant reproduce the bug I was working on (https://code.google.com/p/reviewboard/issues/detail?id=3438)
    • I’ve changed mail backend to console
    • Haven’t enabled registration emails. (This is probably the issue)
    • To enable, go to localhost:8080/admin/settings/email/ or click “E-Mail” in the System Settings group in the admin interface
  • Two Scoops of Django is a good book.

mloyzer

  • I pinged Ryan and we talked about it and he was willling to do another one so i took claim of the one we were both doing (https://code.google.com/p/reviewboard/issues/detail?id=3452)
  • Submitted a RR last night (WIP) (https://reviews.reviewboard.org/r/6322/)
  • The bug says “a warning should be issued that the user did not actually change the RR”. As of right now, all my ‘fix’ does is check if the field was modified and if it was then save it, otherwise don’t. I don’t have any “warning”.
    • m_conley: There are a few ways of solving this problem:
      1. Do not allow the user to publish unless at least one field has changed. If any field changes, and then changes to its original value, revoke ability to publish.
      2. Or allow the user to publish if the change description is empty, but style the empty change description to make it more explicitly clear that nothing has actually changed. So instead of showing this we show some text in there saying, “Nothing was changed”, or something.
    • ChipX86: I kind of think this isn’t an EasyFix actually. There’s the JS side of things, but there’s also the API side. And the reality is, whether or not you’re posting something that looks like a change or not, it’s valid to create a draft with whatever you want even if that draft is empty.
    • purple_cow: I think when publishing we should check (on the API side) whether the draft is empty, and then return a failure, and on the client side, it can show a message when that happens, and that should be pretty easy to do. Just a new API error and then some additional handling on the client side. I wouldn’t try to make the client side figure out whether the draft is empty.
    • ChipX86: you’ll want to look into the ‘publish’ function in reviewboard/reviews/models/review_request.py, and the one in review_rquest_draft.py
Advertisements

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