Meeting Minutes: April 6, 2014

Announcement/General tips:

  • Nearing the end of the course for everyone, CMU has 4 weeks left, TUT has 5 weeks left, MIT has 6 weeks left
  • Think about where you are in the project, how fast you’re moving and the likelihood of you finishing your project given other commitments and adjust accordingly
  • Please don’t stretch work out to fill out time, attack as hard as you can.

Round Robin

Tami

  • tamijo: ok – i think the main things i want to test are: does a review show up, and are reviews on private requests hidden
  • m_conley says those are fine things to test
  • tamijo: I also havent thought about how multiple reviews on the same review request are shown on the grid
  • m_conley: I think each individual review should be shown in the list, personally
  • Tests seem pretty straight forward and tamijo is almost done
  • Tests should have them up and project should be out of WIP by this week
  • Should start browsing the project list for a new project, given how much time is left

Anselina

[Excused: UCOSP Done]

Edwin

  • Put a patch up for sandboxing, it was pretty straightforward
  • Patch is in WIP because he wanted to make sure was on the right path and the tests were written/done correctly
  • Found the hook when m_conley showed when describing the project, also found it by searching through the code and having it on a list
  • Expecting 2 patches per week, so we think you kinda need to up your speed a bit
  • So should we see 3 patches this week
  • m_conley: thanks for writing the regression test.
  • Tests look good, but need to confirm my tests fails without my patch

Olessia

[Excused: UCOSP Done]

Joonas

  • Shoutout to bro1, tomia_, and tamijo for giving feedback on the screenshots
  • m_conley: I think your approach with the dropdown overlay is better than what you had before. It’s not a sexy slide-panel, but for a first version, in order to get something landed, I think we should go for it. I agree that an icon is better than “menu”.
  • this is becoming the icon for “menu”: https://www.dropbox.com/s/1vbq4a3dcu672pi/Screenshot%202014-04-06%2014.21.08.png
  • Screenshot is at: https://reviews.reviewboard.org/r/5620/file/518/
  • The hit area for menu is not quite big enough yet, need to add more padding/make it bigger – those are next steps
  • If we want to use the three-line “hamburger” icon for menu, to get the icon into RB, we have a master icons.svg and export that in normal and @2x versions says ChipX86
  • To get that hamburger into spritesheet, need to use inkscape to edit the file, add the icon in the next available empty space, figure out the coordinates, and make a suitable entry in icons.less
  • ChipX86: you can’t drop a png into the svg file, so it’ll need to be made from scratch
  • m_conley: Joonas_: Inkscape isn’t really a thing that we expect people to be familiar with just yet, so please please please ask for help if you need it with it
  • ChipX86: and if you’re getting stuck doing it, I can put an icon in there
  • m_conley: Joonas_: or just copy it: http://css-tricks.com/three-line-menu-navicon/
  • m_conley and purple_cow agree that the version number is really not necessary to display in the mobile UI, so it can be hidden

Tomia

  • About to take r/5531 out of WIP
  • Realized today that needs_authorization flag does not work with hosting services that act as bug trackers, need to fix that, should be easy enough given it is already done for basic hosting services,
  • tomia_: then I will start adding support for bug trackers, r/5531 is just the foundation. in other words, implement the bugtracker interface for those
  • tomia_: And there are three identical issues open on r/5531, I have been thinking about those. They are about the comments being sent to bug trackers
  • tomia_: I am leaning towards not making them templates like emails, just simple strings as they are. At least I would not like some outside system spamming bug tracker full of 10 line comments. And of course add the localization wrapper for those strings
  • tomia_: https://reviews.reviewboard.org/r/5531/file/520/ the bug infobox is fully functional now, maybe need some css magic

Iines

  • Got the issues from the previous week fixed, though i have to go the style thru
  • Audore thinks she can have this first part out of WIP this week if it’s only about the style
  • ChipX86: I need to do another pass through the actual logic. I still think it’s doing more than it needs.
  • ChipX86: in the meantime, though, spend some time fixing up the style and get that to us ASAP
  • Should be able to get cranked out in an hour
  • Part 2 in the next task, Audore will implement it by showing the code line, which is in the URL
  • m_conley: Audore: alright. I’m looking forward to seeing a WIP for that patch up this week – maybe the earlier the better. Don’t wait for the meeting to roll around.
  • Also closed some of ChipX86’s issues without fixing them, so will need to reopen them
  • m_conley: Audore: OK, so we’d like to see the style fixes up today, and a WIP for part 2 up by Tuesday. Sound good?
  • Audore: well not today, i’m heading to sleep after the meeting, but tomorrow

Mirai

  • Put some more screenshots up as we speak, which shows the /r/:id screen for mobile
  • https://reviews.reviewboard.org/r/5655/file/512/
  • m_conley: Joonas_: you might want to get in there and give some feedback to bro1
  • Has some not so immediately-blocking problems though
  • First thing: uses html meta tag viewport to disallow zoom on mobile devices for the *entire* site, which is rather undesirable because i am currently only working on the review details and was writing some code which wraps this around a django template if, which only inserts for certain urls, but that hasnt worked and was wondering what the correct syntax would be
  • ChipX86: We want all-or-nothing. you won’t be able to really add that per-URL in the base template. you can add it in the more specific templates. but are we adding that just because the top bar and stuff aren’t mobile-friendly yet?
  • bro1: that and perhaps the dashboard. i guess since we are eventually moving most of the stuff to mobile its not a big issue
  • That was more of a curiosity question
  • bro1: One more question, regarding code mirror
  • because bro1 hasn’t been able to get the codemirror to work well on mobile, writing reviews has become much better on mobile (view screenshot), but the cursor does not focus automatically on mobile (probably due to codemirror)
  • bro1: in fact, the user needs to click somewhere else (e.g. ship-it checkbox), and then hit the textarea in order to bring up the keyboard
  • According to stackoverflow, apparently for iOS the browser ignores .focus()
  • ChipX86: so I know they just released a new CodeMirror. Might be worth seeing if it works any better?
  • bro1: ive tried hacking around by adding a click listener as some internet people suggested, but hasnt worked so far
  • m_conley: bro1: maybe you can patch CodeMirror. http://codemirror.net/ says that the maintainer isn’t really targeting mobile, but accepts patches, but that may be getting distracted
  • https://github.com/marijnh/CodeMirror/issues/60
  • Should strive to have code mirror on mobile support
  • ChipX86: we use codemirror in order to provide visual hints for markdown styles. without it, it’s too easy to accidentally write code that will turn into unwanted markdown-formatted stuff
  • Backticks and things like *, _, etc.
  • m_conley: maybe we can work around these bugs. bro1, can you stick around after the meeting to brainstorm?
  • bro1: yeah i have another thing in half an hour but right after the meeting works
  • bro1: regardless of this im going to start tackling the comments section this week

Bhushan

  • bhushan89: Completed testing and development of Bitbucket post-receive hook rr#5653
  • Also resolved the mercurial issue so testing on mercurial is complete
  • bhushan89 just had to add some configurations in ~/.hgrc file
  • Since it’s out of WIP, need to get it reviewed for real
  • bhushan89 wants to get started on the next project this week
  • bhushan89 had a question about Beanstalk testing for next project
  • We have Beanstalk accounts, ChipX86 will give bhushan89 the account details
  • bhushan89: so as David suggested that I can work on this project where the pre-receive hook checks for approval before closing the review request and then closes it automatically
  • ChipX86: It doesn’t close, it just checks if you’re allowed to push. the post-receive hook still handles the actual closing. the pre-receive hook should block the push if you don’t have permission to push
  • m_conley: to blocks pushing on pre-receive if not approved
  • bhushan89: hmm…but won’t that mean taking two trips to the server ? or the pre-receive hook view just calls the post-receive hook view if it is approved ?
  • ChipX86: it is two trips, btu it’s also two stages of a push. pre-receive happens before any code actually gets stored on the server there may be multiple things going on there. They may do things like queue off some tests, or whatever. post-receive happens after everything has been approved and changes are stored. those are the guidelines for repo hooks, and we need to respect them, even if it’s multiple trips to hte server (which aren’t expensive anyway)
  • bhushan89: So I have around one month. I’m planning to get done with this and some more post-receive hooks towards the end.
  • m_conley: bhushan89: awesome. You’ve got a good pace going. Keep up the good work.

Stephanie

  • Finished the front end with dummy image (my own screenshots)
  • Posted a picture of the UI on my WIP
  • hellosjsu has changed the idea a little
  • hellosjsu: originally would have the users click on an X button to remove a widget, but then i noticed the upper right hand corner, already had the minus sign for minimizing. so now add and remove happen in the same place. the user checks off widgets they want to add, and unchecks the ones they don’t want to remove
  • m_conley: that’s slightly “dangerous”
  • ChipX86: I think it’s okay to have the “X”, and it’s contextual, which is good. people can remove it without having to 1) open the menu, and 2) re-locate the widget they want to remove. we can always prompt if they’re sure, but I think it makes the usability nicer to have it alongside the “_”. plus, that’s consistent with how people use windows on basically any OS
  • hellosjsu wondered if it should be in the same corner
  • ChipX86 thinks it’s fine as long as they aren’t bumped up right next to each other
  • hellosjsu: also right now, clicking anywhere on the header minimizes the widget, should we keep that if we do the X?
  • ChipX86 and m_conley don’t get that, only get crosshairs
  • m_conley: hellosjsu: in this case, maybe take some inspiration from Windows’ minimize and close buttons on Windows 7 in terms of clear separation
  • m_conley: hellosjsu: alright – I look forward to seeing the back-end saving code.
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