Meeting Minutes (October 6, 2013)

endee:

  • Working on a fix related to filling the database with mock data
  • Question: Is it okay to write about non-technical things for the student blog post if there isn’t anything super technical to blog about yet?
    Answer: It doesn’t have to be super technical. It can be somewhat technical, or it can be about some issue you had to work with, or a trick you discovered, or whatever

Tweek:

  • Wanted to know how to structure her tests for the security feature she’s working on
    Ideas:
    – Structuring a check as a class would make things nice and clean, and then have a class that’s responsible for running though the checks — kind of like a unit test: a unit test has up to 3 components: a setup function, a teardown, and a runner.
    – The controller’s job would be to know about the different checks and to run each one, storing results for display to the user. Most of that’s going to be independent of Django. Django’s stuff is just for the view function, which would start/run the tests, and the template for displaying the checks and results.
    – The controller is similar to a test runner, instead of a TestCase class. It would be extra neat if this was structured so that an extension could register with this its own security check classes, which would just mean having the ability to add/remove items to/from the list of known checks — kind of like what we do for registering mimetype handlers or hosting services (hostingsvcs/service.py). Security check classes are not extensions, but if they’re just subclasses that the runner can instantiate and run, then an extension could make its own and add them, and they’d run automatically.
    – Something to consider is that some tests, like the file attachment security ones, are going to need to upload files before the test and delete them after. However, if you were to view the same page twice, you’d need to make sure they don’t stomp over each other, so you’d want to, say, namespace them in a directory with something unique.
  • Hackpad link: https://reviewboard.hackpad.com/Security-Checklist-Project-o0Z3CpHwRqU

edwlee:

  • New RBTools command ‘config_repo’ is submitted for review.
  • Will start looking into his next RBTools feature (auto determining previous patch when using post -r) while waiting for reviews.
  • Question: How to properly format Python code in regards to line lengths?
    Answer: For the line length issues, you can wrap the parameters that are crossing the 80 column boundary onto the next line, aligned with the other parameters.
  • Needs to address review request issues that are dropped. Submit a separate patch to fix the ones that are not part of the current patch. A particular issue needs to be addressed is the PerforceClient one as it could cause a crash, as well as issues like “undefined name”.
  • Question: How to properly format long Python if statements?
    Answer:  Use parens around the entire statement and add a newline after operators like “or”, “and”. If the condition is instead one big function call, it’d be best to make that its own statement, storing as a result, and then doing an if on that.

elaineM:

  • Question: Where to initialize a code/file review (e.g. in draftReviewModel or reviewModel) when integrating Backbone.js code with RB code?
    Answer:  You wouldn’t actually be putting any code into any of those for your extension. Your extension can inject into a page through a hook (a template_hook I believe). Your extension would add a hook to render a template of yours inside the diff vieer template. So my first guess at this would be that your extension should render into the file attachment and diff viewer pages through the template hook and that would display the appropriate UI. (need more knowledge of the workflow to the fully answer this question).
  • Question:  If we want the checklists to be preserved across drafts, would the hook also take care of getting the right checklist for the right draft?
    Answer: The hook would just be how you add some script tags to run your new code. That code for the checklist UI would do the work of talking to an API on the server (your API) to get the appropriate checklist contents. That, or you can pass that in when rendering the template. It’s entirely possible that the existing hooks are not going to be detailed enough for what you want to do, so part of your job may be to improve our hook support
  • Main priority now is to use a hook, or create a new one.

classy_baboon:

  • Created the trophy model. Going to create the trophy manager model. Will submit both for review as part of the same change.

OTHER

  • Useful command: In the reviewboard tree, ./contrib/internal/run-pyflakes.py runs checks on the tree and tells you if you have things like undefined names, unused variables/imports. There’s no equivalent for rbtools, so just run ‘pyflakes rbtools’. (run ‘easy_install pyflakes’ if pyflakes is not installed). 
  • Unit tests for RB: there are a bunch of helper methods for helping to write tests that use ReviewRequests, comments, etc. A good file to look at is reviewboard/testing/testcase.py. Your unit test class should subclass that TestCase, and then it can do things like self.create_review_request() to just get a review request to use. The best time to start writing test cases is as soon as you have a single piece of functionality that you know should produce a result given input, or should not produce a result given different input, and you don’t expect to fully rewrite that thing. It’s handy as you develop to have a small test suite you can run after a big change to make sure you didn’t break something.
  • How to run subsets of RB tests:
    https://reviewboardstudents.wordpress.com/2011/09/24/running-subsets-of-review-board-tests/
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