Status Reports (Nov 23rd 2012)

[Jesus Zambrano]

Hey guys, it's my turn to start the status reports:

Jesus Zambrano

Currently:

- Bind ".editable" elements to the inlineEditor() when a user re-opens a review request that was previously discarded.
- Adding a field to RB.ReviewRequest to be able to check whether a review request is public or a draft.

Roadblocks:

- I'm not really sure how to go about doing point #2 above. So far(As told by ChipX86), I've created a function _loadDataFromResponse() in RB.ReviewRequest that gets called in _checkForUpdates(). But i'm not sure how that helps me when trying to check whether a review request is public or a draft.

Next:

- Solve Roadblocks

Questions:

- None

[Allyshia Sewdat]

Allyshia Sewdat

Currently:

- Updated JSLint patch as a major review was done (Thank you Mike), made some changes to functionality -- awaiting feedback on my counter-comments.

- Did some reading about extensions + hooks in reviewboard

- Discussed design with smacleod regarding manual trigger

Roadblocks:

None

Next:

- Add that UI and hook up the manual trigger ASAP, considering future permissions concerns

Questions:

- Would be appreciated if I could get some final comments on the questions I asked in the  JSLint review #3435. Thanks!

[Aamir Mansoor]

Aamir Mansoor

Currently:

- Finished addressing issue for "Markdown Pluggable UI". Would love any more feedback on it

- Addressed most of the issues for "WIP - Rendering pluggable UIs in lightbox". Still working my way through it.

- Had issues with git because of the way I was merging/rebasing things (Thanks Christian and Mike for helping me fix those!)

Roadblocks:

None

Next:

- Continue working on rendering review UIs in lightbox

- Hopefully get a "Ship It" on Markdown Pluggable UI over the next few days

Questions:

None

Aamir Mansoor

http://aam1r.com

[John Sintal]

John Sintal

Currently:

- Finished setting up svn on local environment and dev server, able to post reviews with svn

- Working on rb patch

Roadblocks:

- pX arg for svn -- a really morale killer here, been struggling a lot. Definitely impeding me.

- patch just isn't working (see question below)

Next:

- get pX logic working with svn

- talk to steven about the options that aren't needed

Questions:

(Please excuse this huge question)

Patching works fine if I create the diff manually.

i.e make some changes, 'svn diff > diff.txt', revert changes, then run 'patch < diff.txt'

However, this way fails consistently:

Steps

1. svn checkout https://texttc.googlecode.com/svn/trunk/ in some folder, called /repo1

2. Go to trunk/src/opl/textc and edit Find.java, and save that file

3. post-review to dev server and publish. Verify the diff is correct (changes to Find.java)

4. svn revert -R . (put things to before state so as to apply a patch).

5. rb patch <rid>

6. We get a command like this ['patch', '-p0', '<', '/tmp/tmpzDcnC9']

-- doesn't work, i've tried with -pX, -p0 to -p5, all fail, what am I doing wrong? ---

Error Msg:

patching file Find.java

Hunk #1 FAILED at 1.

1 out of 1 hunk FAILED -- saving rejects to file Find.java.rej

Here are some state variables

repository_info.base_path:

'base_path': '/trunk/src/opl/textc'

pwd:

/home/user/repo1/trunk/src/opl/textc

contents of downloaded diff:

Index: /trunk/src/opl/textc/Find.java

===================================================================

--- /trunk/src/opl/textc/Find.java (revision 131)

+++ /trunk/src/opl/textc/Find.java (working copy)

@@ -1,4 +1,4 @@

-package opl.textc;

+package opl.textcooo;

 

 import java.io.InputStream;

 import java.util.ArrayList;

Command attempts (w/o Python)

patch < /tmp/tmpfB7d8S

patch -p0 < /tmp/tmpfB7d8S

...

patch -p5 < /tmp/tmpfB7d8S 

=(all fail, giving a msg like so)==> 

patching file Find.java

Hunk #1 FAILED at 1.

1 out of 1 hunk FAILED -- saving rejects to file Find.java.rej

[Tina Yang]

Tina Yang

Currently:

- Fixing the issue where the table cells are not aligned properly in Chrome

- Refining the UI for inline file attachment

Roadblocks:

- None

Next:

- Found out that sidebyside text diff cells are not the same size, trying to resolve that by making them the same size and aligned with the inline file attachment sidebyside view.
( Issue explained: http://i46.tinypic.com/eiumw8.png : notice how the lhs of the text diff table cell is a bit wider. For this image in my Chrome browser: both the cells for the inline file attachment are 782px wide; but the lhs of the text diff (col #2) is 777px wide but the rhs (col #4 - AKA the new file side) is only 747px wide. Unless this is by design, I will attempt to fix this. )

Questions:

- We can't delete a file attachment from a review request yet, is that correct? If so, is there any tips on clearing up the review request? I have a lot of screenshot files on my review requests that are no longer applicable, I think they make the review request somewhat confusing and want to clean them up somehow. Would labeling them by revision number in the caption help? Any tip on good practice in this case?

[Wesley Ellis]

Wesley

Currently:

- Addressing issues raised in Buildbot plugin review
- Trying to figure out a sane way to supply per-worker configuration

Roadblocks:

- None

Next:

- Getting SSH buildbot integration working
- Get review branch and pass it to buildbot
- Make the output prettier

Questions:

- Is there any kind of comment formatting that I can use to colour the output of the build?

Wesley Ellis
http://about.tahnok.me

On Sat, Nov 24, 2012 at 12:34 AM, Tina Yang <tinay...@gmail.com> wrote:

Tina Yang

Currently:

- Fixing the issue where the table cells are not aligned properly in Chrome

- Refining the UI for inline file attachment

Roadblocks:

- None

Next:

- Found out that sidebyside text diff cells are not the same size, trying to resolve that by making them the same size and aligned with the inline file attachment sidebyside view.
( Issue explained: http://i46.tinypic.com/eiumw8.png : notice how the lhs of the text diff table cell is a bit wider. For this image in my Chrome browser: both the cells for the inline file attachment are 782px wide; but the lhs of the text diff (col #2) is 777px wide but the rhs (col #4 - AKA the new file side) is only 747px wide. Unless this is by design, I will attempt to fix this. )

Questions:

- We can't delete a file attachment from a review request yet, is that correct? If so, is there any tips on clearing up the review request? I have a lot of screenshot files on my review requests that are no longer applicable, I think they make the review request somewhat confusing and want to clean them up somehow. Would labeling them by revision number in the caption help? Any tip on good practice in this case?

On Fri, Nov 23, 2012 at 4:26 PM, John Sintal <john....@mail.utoronto.ca> wrote:

[Sampson Chen]

Sampson Chen

Currently:

- Implemented mem caching for thumbnail project on .rst and .md attachments

- Refactored the Text-ish classes in mimetypes.py to remove duplicate logic; ReStructuredTextMimetype and MarkDownMimetype now subclass TextMimetype instead.

- Implemented infrastructure for registering MimetypeHandlers in much the same manner as registering Review UIs, replacing the old way of doing it through __subclass__

- Implemented protection against js injection attacks while using docutils / markdown modules

Roadblocks:

- None atm, just putting a lot of time into to resolving all the loose ends for thumbnails project to get it ready for patching, so I can get back to the extensions project.

Next:

- Test some mockups with the extension project

Questions:

- None atm

[Karl L.]

Karl Leuschen

Currently:

- Cleaned up remaining issues and landed patch dealing with checking if SSH keys already associated.

- Incorporated patch into WIP review request

- Modified WIP request (r/3488) based on feedback from Christian

- WIP request is functional, but still needs review before it can be moved out of WIP

Roadblocks:

- All clear.

Next:

- Fix issues with r/3488

- Reviews

Questions:

- None for now

[Michelle C]

Michelle Chuang

Currently:

- Working on getting images overlayed for a Swipe and Onion image diff view

- Adding functionality for Swipe view

Roadblocks:

- All clear.

Next:

- Posting a code review ASAP

Questions:

- None for now

[David Trowbridge]

Here are some answers to the questions so far:

John:

> Patching works fine if I create the diff manually.

> i.e make some changes, 'svn diff > diff.txt', revert changes, then run 'patch < diff.txt'

> However, this way fails consistently:

> ...

Given the diff content you pasted in and the repository you linked to, I'd have expected it to work with -p2 (stripping off "/trunk/"). Are you sure your working directory is what you expect?

Tina:

> We can't delete a file attachment from a review request yet, is that correct? If so, is

> there any tips on clearing up the review request? I have a lot of screenshot files on

> my review requests that are no longer applicable, I think they make the review

> request somewhat confusing and want to clean them up somehow. Would labeling

> them by revision number in the caption help? Any tip on good practice in this case?

The red "x" on the attachment should remove it from the request. If for some reason

you want to keep them around, it helps to change the captions to indicate which ones

are current (and yes, perhaps labeling them by their corresponding diff revision).

-David