Meeting Minutes (Oct 27, 2013)

general announcements:
– only a little over 1 month left in the course, stay organized and deal with the final month well

endee:
– I’m currently working on trying to trigger the inline editing next to the people’s field to activate when clicking on a “suggestion” link that i have showing up.
– <ChipX86> : so the widget the inlineEditor is registered on can have commands invoked on it you can initiate editing by doing: $(element).inlineEditor(‘beginEdit’). That said, we may want to have a nicer way of doing this. ReviewRequestEditorView knows about every editable field giving it a command for saying “start editing this field” would be good
The $(element) would be #target_people in this case
so you would need:
1) to add a function on ReviewRequestEditorView. Soemthing like “beginEditField” that takes a field name. Maybe a “setFieldValue” as well, and “getFieldValue”, for setting the contents
2) access the ReviewRequestEditorView from your code. You can get this through RB.currentPage.reviewRequestEditorView, iirc

elaineM:
– m_conley looked through her review request…

– elaineM: href=# refreshes the page on click through chrome, so she uses Javascript:void(0).
– ChipX86: return false from the callback handler to prevent default action
– How do I get an ID for a review/review-draft:
– A. in the template {{review}} variable references pending review, if one is set the data can come out of there to be passed to the javascript. For example; review.id. Also check for review = None.

– elaine: if I’m on a diff, it doesn’t mean there’s a review?
– Answer: no, not unless you start writing one. The {{review}} variable will either be None if there is no pending review or an object if there is one. You have a review request not a review.

– Checklist should show up before a reivew. A reivewer makes the checklists so that they can remember things to look for, then we save it and leave it. The checklist can then be decoupled from the review and remain with the review_request + user alone.

– elaine: is there a time when a user can review one review request more than once? answer: yes.
– Would the checklist be the same? Answer: yes, leave it up to the user when to restart a checklist if they are reviewing a single reviewrequest multiple times. On the backend, the checklist will have foreign key relationships to both the User and the ReviewRequest, and then a unique_together constraint. One active checklist per user per review request.

-m_conley: It might be worth writing up how all of these pieces work together, to help make sure we’re all on the same page… would make good blog fodder.

 

edwlee:
– has r/4662 waiting on purple_cow to look at
– purple_cow is good with “setup-repo” if that’s what ChipX86 and smacleod say.
– ChipX86: your instinct about changing code formatting issues is the right one. Those should be left to a separate review request, unless it’s on lines you’re already working with.

– Edwlee’s status report question:
– Answer from m_conley: avoid getting distracted with style fix-ups that are unrelated to the thing that a review request is trying to address

-Edwlee: as far as expectations go how many more features should I finish for rbtools? I’ll be starting on my 3rd one this week.
– RBTools projects are nice, small and atomic… Do you want to continue working on features or change it up..
– Edwlee: I’m comfortable with it but also open to ither things. I want to finish finish the one for rbt patch and perhaps one more, but I need to think about it some more.

 

classy_babboon:
-In my review request for the Trophy model, it is mentioned that there will need to be evolutions for changes to existing models. Would that include foreign keys in ReviewRequest and User to Trophy?
– m_conley: ChipX86 saved the day there – you shouldn’t need an evolution when creating new models… you only need evolutions when you *modify* models
– classy_babboon: but ChipX86 mentioned after that for other models, there will need to be modifications
– ChipX86: if you were to add a field, so if Proflie had a ManyToManyField for Trophies then yes, or if ReviewRequest did.
– classy_babboon: There shouldn’t be any ManyToManyFields
– ChipX86: the rule is: any changes to new models you introduce do not need evolutions. Any changes to other models do. if the latter case, the doc m_conley linked will come in handly.

– class_babboon: I’m wondering if ReviewRequest and User would possibly need a foreign key to Trophy in their table
– ChipX86: A ForeignKey would be wrong, a ForeignKey would indicate that it can link to exactly one Tropy.
– m_conley: you’ve already got the relationship set up with your Trophy model, like the “related_name” is what you’d use from a User, for example, to access the many trophies.

– ChipX/m_conley: post a WIP for trophy assignment logic of whatever you have, doesn’t have to be pretty.
– classy_babboon: it’s written on paper, will try and get a WIP up on Monday

 

Tweek:
– Technical Question: I’m sending a dictionary to the front end: It’s entries look like: ‘executable_check’ : {‘name’: ‘Executable Code Check’, ‘result’: True } (Passing it through render_to_response) . On the front end I’m iterating over it with {% for check in test_results %} saying {{ check }} works, But {{ check.name }} and {{ check.results }} come up blank

– ChipX86: I believe that’s only returning keys.. What you can do is {% for key, value in test_results.items %}. Y
– purple_cow (and chipX86 agrees): You may also want to change it to pass in a list of checks rather than a dict, since dicts iterate in somewhat random order.

-tweek: So as another check i’m thinking about checking whether or not the admin is running rb on a web server that is too old. Can you think of a sane way of checking this?
– purple_cow: there might be something useful in the environment.
– chipX86: there’s a header you have access to in request.META for the Apache version, if running Apache.
– m_Conley: it’s probably going to be pretty different from server to server.
– ChipX86: the main checks I’d concentrate on right now are the ones for file attachments, that’s a real world thing that we had trouble with.
– Tweek: worried that 2 checks is kinda short
-m_Conley: 2>0.
– Tweek: Current checks: file attachments (making sure that if you upload something like a php file, it isn’t getting executed.. It’s checking by actually fetching the file and comparing.), and allowed hosts.

 

 

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