- m_conley (Mike Conley)
- ChipX86 (Christian Hammond)
- purple_cow (David Trowbridge)
- heapify (Anselina Chia)
- smacloed (Steven McLoed
- brennie (Barret Rennie)
- dkus (David Kus)
- nicole_xin (Nicole Xin)
- andrewhong (Andrew Hong)
- rdone (Ryan Done)
- asalahli (Azad Salahli)
- justy777 (Justin Maillet)
- <m_conley> Hello everybody! This is it – this is the last official meeting of the semester for UCOSP
<m_conley> (yay – you made it! 🙂 )
<m_conley> Before we go into the round-robin, I wanted to quickly talk about “what happens next”
<m_conley> After Pencil’s Down, we write up your final evaluations
<m_conley> your final evaluations from us are then sent to the UCOSP steering committee
<m_conley> who then are responsible for sending those evaluations to your home faculty
<m_conley> your final evaluations have more than just a numeric grade – we often write textual feedback in there
<m_conley> stuff that we intend for you to see – so if you end up only getting a number from your home faculty
<m_conley> pester them to get text bit too
<m_conley> we’ve heard that some students in the past have only gotten the numeric grade
<m_conley> so, just keep an eye out for that. I’m not sure what the timing is on the release of those grades – I think that’s up to your home faculty
<m_conley> Once your evaluations are in, we hope you join us for the demos / post-mortem session, which I’ll send mail about sometime this week
<m_conley> And then, well, after Pencil’s Down, your responsibilities for this course more or less cease
<m_conley> we’d love…*love* for each and every one of you to continue contributing to Review Board
<m_conley> you’ve all gained skills
<m_conley> you all are now experienced in various sections of the codebase
<m_conley> and some of you might have some final bits to do to push your project into a release state
<m_conley> that’s totally fine, and we *encourage* you to continue hacking on Review Board
<m_conley> and even if your project is put away, we can always hook you up with more work
<m_conley> we’re going to have another wave (actually, waves) of students this upcoming semester
<m_conley> and they’re going to be starting where you started from – from more or less zero
<m_conley> and so giving guidance feedback to them is super useful, and something we encourage
<m_conley> ChipX86 mentioned this last week, that we’re going to try an experiment
<m_conley> where we transition from this IRC channel to Slack for communications
<m_conley> so if you are interested in continuing to hack on RB, we can give you access to our Slack instance
<m_conley> and you can hang out in the #alumni channel, and the #students channel to help out
<m_conley> so if you’re interested in that, let us know, and we’ll hook that up
<m_conley> I’ll also point out that you’re under no obligation to continue hacking on RB
<m_conley> but if you like the project, we encourage it. 🙂
<m_conley> I think that’s my shpeal
<ChipX86> even if you’re not going to hack but want to keep in touch
brennie (Barret Rennie):
- regarding open thread on commit history items:
- <ChipX86> there’s a lot of feedback in there for many different scenarios
<ChipX86> a v1.0 of this doesn’t need to implement it all, so long as we can move toward those things later
<m_conley2> careful not to get bogged down by scope creep
<m_conley2> something like this gets people excited
<m_conley2> and they start asking for the moon
<ChipX86> the majority of people will get benefit from just being able to post multiple commits
- <ChipX86> there’s a lot of feedback in there for many different scenarios
- <brennie> right now I’m just trying to get the first parts that I mentioned in the last meeting working
<m_conley2> brennie: ok. Anything we can help unblock you on?
<brennie> But for some reason, the API endpoint doesn’t show up in the /api/ resource’s uri list
<ChipX86> the uri_templates?
<brennie> I *think* its supposed to show up there — but I’m not sure
<ChipX86> I can help you debug that in a bit
dkus (David Kus):
- <dkus_> Nothing new outside the status report. Continuing to look at reviews on my review requests. I’d also like access to the Slack instance so I can keep in touch & add any finishing touches to my project.
<m_conley2> dkus_: thanks for responding to my feedback so quickly – those patches are looking pretty good
- <ChipX86> if it’s a user-visible error, use alert() for now
<ChipX86> it sucks, but it’s what we have
<ChipX86> console.error will be silent on IE if the console wasn’t previously opened
- <dkus_> If anyone else wants to review my changes go ahead. But I won’t have much time these next few days to work on this.
mloyzer (Mark Loyzer):
- <mloyzer> cool, i don’t have anything new from my status report
- <m_conley2> mloyzer: so you’re going to look at the browser devtools to see what might be eating your click event
<mloyzer> besides that we talked briefly before the meeting, so i’ll try to debug the issue
- <m_conley2> heapify / smacleod / purple_cow / ChipX86: Check out https://reviews.reviewboard.org/r/6633/
- <m_conley2> I wonder if perhaps that should be something that’s just included in the extension
<ChipX86> I have thoughts
<m_conley2> I just feel a bit weird having something so specialized get added to the hook list
<ChipX86> I think the handler for the action buttons is unintentinoally blocking any custom actions
<ChipX86> we don’t want a custom hook for this though
<m_conley2> ChipX86: oh, you’re talking about what’s eating the click event?
<ChipX86> just realized this is different
<mloyzer> has the ReviewRequestDropdownActionHook ever been used?
<ChipX86> but no, we don’t want a custom hook. The reason we have get_actions() is so that subclasses can do things like this
<m_conley2> ChipX86: so mloyzer should move his subclass into his extension?
<ChipX86> mloyzer: once upon a time, but we haven’t done any real testing with it in a while
<ChipX86> and should just return the entire result inline, rather than have an ‘actions =’ in the class
<mloyzer> ah ok, so move it into my extension and debug then?
<m_conley2> mloyzer: so no biggie, but just move the subclass into your extension somewhere so it ships with it instead of putting it into core.
<mloyzer> ya, makes sense
<m_conley2> mloyzer: cool – then yeah, check out the event handlers on the dropdown to see if something’s eating it. I think purple_cow ran into something similar at the sprint with something he was working on…
<m_conley2> I can’t remember how he resolved it
<m_conley2> mloyzer: but he might get some feedback (he’s on the road right now, so his connection is spotty, but we can ask later)
nicole_xin (Nicole Xin):
- <m_conley2> sounds like you’re ready for some real reviews o https://reviews.reviewboard.org/r/6623/
<m_conley2> nicole_xin: can do!
<m_conley2> nicole_xin: awesome – I’ll try to get some in tonight
<nicole_xin_> everything is ready for reviews
<nicole_xin_> oh thanks
<m_conley2> oh awesome, yes
<m_conley2> everything for you is out of WIP
<m_conley2> so we’ll get reviews for you ASAP
rdone (Ryan Done):
- <m_conley2> sounds like you’re also ready for some review on https://reviews.reviewboard.org/r/6591/
- <rdone_> theres still a few changes I’d like to make
<rdone_> but the core slider functionality is there
<m_conley2> ” Is the appearance of the text attachment slider satisfactory (note the shadow)? If not, what can be done to improve it? CSS changes?”
<m_conley2> yeah, I’m not the biggest fan of that shadow
<m_conley2> can we just flat out remove it?
<ChipX86> the slider needs to be toward the top too
<m_conley2> right – above the diff?
<m_conley2> I guess that makes sense – you don’t want to have to scroll down all the way to the bottom to change it
<ChipX86> I’d put it in the same visual box as the “test1.txt – test1”
<ChipX86> maybe right above it, or below it, not sure
<rdone_> I think there should also be a title to the right
<rdone_> for the diff’d file
<rdone_> “test2.txt – test2”
<rdone_> but I haven’t gotten to that yet
<ChipX86> yeah, we should be as close as possible to the diff viewer for diffing
<m_conley2> I’d put it above those two titles
<rdone_> at the moment its using this.$el.append(“”)
<rdone_> and it sticks it at the bottom
<rdone_> is there a a different call for above?
- <m_conley2> ” I’m still unsure about how to manage swapping between “diff mode” and “view single file mode”. But I have a suggestion: Perhaps a button or control that allows the user to switch between 2 handles on the slider (for diffing) and 1 handle (for viewing each revision).”
<m_conley2> I thought we resolved that last week, didn’t we?
<rdone_> not really
<rdone_> the response was unsure, so I figured an image or implementation would help
<ChipX86> we should use what we have in the diff viewer
<ChipX86> I thought we solved that with that but a different label suggested by purple_cow
<ChipX86> (he’s on the road so he can’t weigh in)
<ChipX86> it’s very important for users to be able to use this feature without learning something new
<rdone_> what does the ‘orig’ tag in a file attachment history?
<m_conley2> the more we can crib from the diff viewer, the better
<rdone_> the context is sort of different from a code diff
<ChipX86> it is different. purple_cow had a recommendation on it I think
<rdone_> what does the orig tag *represent…
<ChipX86> maybe it was “new” or something
<rdone_> purple_cow: I think “orig” is a really weird thing to say for file attachments, thought
<ChipX86> we can figure out the label later. It’s the concept that matters
<rdone_> Well I can add the label with a line or two
<ChipX86> if a user goes in the diff viewer, uses hte slider, and learns how it works, then goes here and sees that the same slider for the same concept works differently, the user will grumble
<m_conley2> same affordance should behave the same way
<ChipX86> changing the label can be done by us down the road before release, if need be
<ChipX86> so long as the behavior is identical
<rdone_> What should happen when a file attachment review is first visited
<rdone_> display 1 file?
<ChipX86> just as today
<rdone_> and the slider is there with 2 handles, and they move on and get 2 files back
<rdone_> how do they ever return to single file mode
<ChipX86> they click the label for the revision they want, or select a range from “orig” (or whatever) to the desired revision
<ChipX86> identical to the diff viewer
<rdone_> gotcha, so labels for selecting individual files
<rdone_> and slider for selecting 2
<ChipX86> it’s all part of the same slider
<rdone_> in that case the ‘orig’ tag (or w/e its called) is purely cosmetic
<rdone_> to identify the first or last revision
<m_conley2> cosmetic, but also familiar
<ChipX86> just the one label on the left, before “1”
<m_conley2> rdone_: make sense?
<rdone_> would it still point to the first revision
<rdone_> and then ‘1’ would point to the next one
<ChipX86> “orig” doesn’t point to a revision
<ChipX86> it points to a state before revisions
<rdone_> original file attachment
<ChipX86> well, “orig” needs to be renamed later, but it’s the conept
<ChipX86> it’s “none”
<ChipX86> if I have a range from the “orig/none” to “5”, then I’m viewing revision 5
<ChipX86> clicking the “5′ label puts it into that state as well
<rdone_> yeah that makes sense
<ChipX86> there are definitely things to iron out here, but it must be done along with changes to the diff viewer
<ChipX86> and that’s on our plate 🙂
- <m_conley2> “ChipX86 mentioned last meeting that we needed to make sure my code would not impact the diff viewer (image review ui in the diff viewer I believe), but I haven’t had a chance to look into this at all….”
<m_conley2> rdone_: I guess that’ll get sorted out during review.
asalahli (Azad Salahli):
- <asalahli> So, I was going to ask for more reviews, but that got solved a few minutes ago 😀
<m_conley2> asalahli: heya – yeah, I just saw that myself. 🙂
<m_conley2> asalahli: any questions about what you’re seeing in that review?
<asalahli> 2 things
<asalahli> 1 is just a grammar issue
<m_conley2> asalahli: go ahead
<asalahli> don’t we “land something on something else”?
<asalahli> in this case, land changes on a repository/branch
<asalahli> or to a repository/branch
<ChipX86> the first comment?
<asalahli> I’m talking about the first review here https://reviews.reviewboard.org/r/6509/#comment19122
<ChipX86> may just be me
<asalahli> oh I see the point now
<asalahli> should I change it?
<m_conley2> you could try “Land changes from a review request onto the remote respository”
<ChipX86> there we go
<m_conley2> best of both worlds
- <asalahli> And second thing
<m_conley2> asalahli: what’s the next question?
<m_conley2> go ahead
<asalahli> can we decide on option names for source and destination branches?
<asalahli> thats the only thing blocking me right now
<m_conley2> what’s wrong with –source-branch and –destination-branch ?
<ChipX86> I don’t recall exactly, but I thought last week there was a suggestion to have the local branch be the first argument?
<asalahli> ChipX86: yeah purple_cow suggested that
<ChipX86> the destination branch will largely be in .reviewboardrc, so if the command line option is a little longer, not a big deal. It won’t be typed as often
<asalahli> but I had a question and we couldn’t discuss it
<m_conley2> asalahli: what’s the question?
<asalahli> m_conley2: –source-branch will be typed pretty often, so I thought its a bit long
<m_conley2> often those arguments have single char counterparts
<asalahli> Ok. so purple_cow’s suggestion:
<m_conley2> –source-branch == -S
<asalahli> two different types of usage
<asalahli> but my concern is that, there are times when a user needs to specify both rr-id and src-branch
<asalahli> btw, I like m_conley2’s suggestion
<ChipX86> keeps it consistent with tools like git and with post-review
<ChipX86> rbt post 😛
<ChipX86> with rbt post, you specify an ID with -r
<m_conley2> consistency is good – especially for grouped tools like this
<ChipX86> and you can specify branches to post with a positional arg
<ChipX86> git merge, etc. also use positional args
<asalahli> and to land current branch just type ‘rbt land -r <id>’?
- <m_conley2> asalahli: any other questions
<asalahli> should I name the other one -D, –destination-branch ?
<ChipX86> I don’t know that we need short-hand for this one
<ChipX86> if we do, I think –dest or –dest-branch is fine
<asalahli> I like –dest more
<m_conley2> do it
- <m_conley2> asalahli: anything else?
<asalahli> Thats it. I was supposed to implement more backends, but didnt have time this week. Apologies for that 😦
<m_conley2> asalahli: that’s alright – perhaps you can do it after the course is over. 🙂
<asalahli> that was my initial plan 🙂
<asalahli> thats it from me today
andrewhong (Andrew Hong):
- <andrewhong> Hello! No updates from me since the status update, still awaiting reviews though. I would like to get on the Slack list as well when it becomes available 🙂
<m_conley2> so you’re blocked on reviews
<ChipX86> going through them today
<m_conley2>> andrewhong: cool, thanks. 🙂
<andrewhong> yeah, the tarball feature was still wip until this week
<m_conley2> glad you got it out of WIP – good job.
<andrewhong> so both should be good to go now 🙂
justy777 (Justin Maillet):
- <m_conley2> great job on all of those PEP8 issues
- <m_conley2> you ask how to build the docs
<m_conley2> smacleod / heapify / ChipX86: do any of you remember that command?
<m_conley2> I’ve honestly never done it, but I know there is one.
<ChipX86> just go into the docs and type “make html”
<m_conley2> right – there seems to be a Makefile for each folder under docs
<ChipX86> you’ll need Sphinx if you don’t have it
<ChipX86> easy_install Sphinx
<ChipX86> I think our Get Started guide has it
<justy777> simple, I like it
<ChipX86> er, has the list of things needed
- <justy777> Does anything in reviewboard call the webAPI or is it something for third parties?
<ChipX86> we do
<ChipX86> we use it for most operations in the web UI
<m_conley2> justy777: a bunch of the front-end uses it, and so do the client tools
<m_conley2> it’s used pretty extensively
<justy777> okay, does anything grab the server info?
<ChipX86> RBTools does
Last but not least… 😦
<m_conley2> alright folks
<m_conley2> well, that’s… that’s it I guess. Stay tuned for my email about the demos and post-mortem
<m_conley2> but officially speaking
<m_conley2> this is it, except for the interactions we have on IRC, mailing lists, or RB until Wednesday
<m_conley2> and well, any interactions we have with you on the project afterward. 🙂
<m_conley2> Thank you so so so much for all of your hard work this semester
<m_conley2> keep pushing through to this Wednesday, and make sure you get those reviews
<m_conley2> You were great! *applause*
<heapify> yup, thanks everyone – great job this semester! 😀
<smacleod> Thanks everyone 😀
<ChipX86> seriously guys, this was a good semester
<mloyzer> Thanks guys
<andrewhong> thank you all! 🙂
<brennie> 🙂 thank you very much
<asalahli> good? this was awesome 🙂
<asalahli> Thanks everybody 🙂
<ChipX86> (was telling my girlfriend yesterday how sad I am that it’s over already)
<justy777> Group Hug?
* asalahli hugs
* ChipX86 hugs
<m_conley2> IRC hug
<justy777> \me hugs
* justy777 hugs
* smacleod strains to wrap his arms around the large group hug
* brennie hug
<m_conley2> uh, I guess we’ll all see you in Slack too. 🙂
<m_conley2> Because basically everybody requested accounts
<m_conley2> so, see you there. 🙂