Author Archives: edwleeca

Status Report – Nov 23

Allisa

* What you’re currently working on:
Implementing changes from my past review.

* What you’re working on next
Finishing other test and prettying up frontend.

* What’s blocking you from progressing
Nothing

* Any questions you might have
None

Natasha

* What you’re currently working on:
Updating my back end query to get all the information I need efficiently

* What you’re working on next
Going to finish this today and post a review request for it.

* What’s blocking you from progressing
Nothing

* Any questions you might have
None

Behzad

* What you’re currently working on:
Creating the TrophyType class to use to connect the backend to the frontend.

* What you’re working on next
Creating stubs for subclasses of the TrophyType in order to get the frontend displaying correctly.

* What’s blocking you from progressing
None.

* Any questions you might have
None.

Elaine

* What you’re currently working on:
Fixing up the checklist code as suggested by my reviewers.

* What you’re working on next
I’m thinking of improving the aesthetics a bit, like maybe getting rid of the add button altogether, and have users add items by pressing enter.

* What’s blocking you from progressing
None

* Any questions
There’s this weird bug that happens sometimes; after refreshing my page a few times, I noticed that my checklist will get rendered multiple times. And then I noticed that the page is making multiple POST requests. It doesn’t happen always though, only after I’ve refreshed maybe six or seven times. How do I stop it from doing this?

Edward

* What you’re currently working on, or have just completed:
I just completed indexed search with Haystack and Whoosh.

* What you’re working on next
Reviewing my own review request to check for issues and possible improvements.

* What’s blocking you from progressing
None

* Any questions you might have
None

Advertisements

Meeting Minutes: November 10, 2013

Annoucements

m_conley: Now’s about the time when you should be battening down the hatches and trying to get your patches out of WIP. At least, beginning that process. You still have ~1 month remaining. but that time flies by. It’s more like 3 weeks, anyhow. We’ve been saying this week after week, but it’s worth saying again. If any of you are worried, are stuck, or otherwise feel like you’re really on the wrong track – please come talk to us.

ChipX86:  Given where we are, it’s best to start getting as much done as possible, getting WIP requests out frequently. If you have anything you haven’t posted yet, put out one today, and put in the description what’s done and what needs to be done still, so we know where you are. There’s only a few weekly meetings left.

Round Robin:

endee

There has been some performance issues with queries. There are some Django tools that help identify slow or sub-optimal queries. django-debug-toolbar is a good one.

Tweek

[how do you feel about your projects progress? Any lingering questions or issues?]
-Overall I feel like I’m a little behind (like I could have had more done up to this point) but after getting the basics down for the file uploading test, I’m not concerned about moving forward. Like it was a lot easier than I thought it would be + took a lot less time.

[do you feel the code is at a point where, as it is, it could be checked in (after a round of reviews)?]
-I think it would probably need a couple hours of polish before I’d want to check it in. Just to move it from “This is sort of working” to “This would be scaleable”

[also, not to get you off track from your project, but there was an EasyFix you’d assigned yourself]
-I’ll get that done tomorrow as well.

-In terms of uploading/DLing a file right now I’m not sure where I can find the address of the current server I’m on?
There’s a Site object that has it. If you grep for Site.objects.get_current() and ‘domain’, you’ll find it. We have to build these URLs in other places as well. Here is how Review Bot does it: https:/githubcom/reviewboard/ReviewBot/blob/master/extension/reviewbotext/extension.py#L105

elaineM

[You’re tackling creating a WebAPI resource – pretty intense stuff, isn’t it? Lots of documentation in that WebAPIResource thing to wrap your head around]
-It is, like I think I lagged behind a week or so, just because it’s such a beast. I thought I was going to get the API stuff done this weekend.

(ChipX86 On WebAPIResource)
List resources are things like a list of review requests, and item resources are like a single review request.
item_child_resources and list_child_resources build that tree.
item_child_resources are child nodes of the items that the WebAPIResource represents.
list_child_resources are the child nodes of the list itself.
So, review-requests has a list of reviews as a child.

purple_cow: (it’s maybe worth pointing out at this point that a WebAPIResource often provides both the list and the item implementation in one class).

To make that happen, we put ReviewResource in ReviewRequestResource.item_child_resources.
Very rarely does a list have specific children, so mostly you care about item_child_resources. You may not even need them for your implementation.

If a Checklist doesn’t have any child resources, you can ignore these when you set Extension.resources, we basically take that and turn it into an item_child_resources on your extension’s own resource, which is how you get that link.

The tree is built up of links. We also use those relationships to build the URLs.
Since WebAPIResources know how to construct URLs by themselves, you don’t need URLHook.
URLHook is a way to tell Review Board that you’re creating an arbitrary URL somewhere

-What kind of testing do you guys expect to see from this extension?
given where we are, I’m not expecting a full unit test suite, but if you can write some unit tests, that’s always good

edwlee

-I’ve started working on a new search implementation using Haystack with Whoosh. I have a basic, working proptype that indexes Review Requests on a couple of fields. I’d like to try the current search to get an idea of how to proceed with the prototype, but I don’t have PyLucene configured. I’d like to try it in a vm instead of configuring PyLucene on my current env. Does our vm have it configured?
No. But messing with all that inside a linux VM could be easier than on your OSX box anyways

[so, what exactly are you looking for now? a list of things to index?]
-Yes, that and how search is used from the user’s perspective

Doc on full text search: http://www.reviewboard.org/docs/manual/dev/users/searching/full-text-search/

-I’ve left this issue open: https://reviews.reviewboard.org/r/4902/#comment12667. I’ve tested it and it seems okay. Just wondering if it can be closed or if i should do some more testing.
ok, if that’s the case, great / sounds like it can be dropped

classy_baboon

-I feel like im a bit behind. I have some code for which i need to post a review request. my unit test still isn’t working and i’m not sure why, i’ll investigate that more. when i get that going, a review request would create a possible trophy and assign it to the review request, user and local site. it is all hard coded though. i’m not sure what my next step should be. should it be to 1) connect it to UI, 2) to make it so that all the review requests of someone are checked when their profile is visited, 3) to make it abstract so that it will work with extensions or 4) something else?

[what do you mean by “it is all hard coded though”?]
-well, it is calculating for milestone and palindrome, but as ChipX86 pointed out, we want to get rid of that eventually, and have extensions dictate the trophy types and their logic

[…that really should be 15 minutes of work though. It’s mostly copy/paste from the other parts of the codebase I pointed out. This extendable pattern is very common in Review Board. This is a very core part of what we want from the new trophy implementation.]
-i’ll do it just as soon as the unit tests pass so that i’m sure everything is fine

-Since rbt post takes an -r argument for the review request to which this commit will be added onto as a diff, would that be “rbt post -g -r <id>”?
You can leave off the -g unless you’ve changed your description

Status Reports – October 19, 2013

Allisa Schmidt

* What you’re currently working on, or have just completed
I finished a backbone of sorts for security tests to be run.

* What you’re working on next
Fleshing out the above.

* What’s blocking you from progressing
Nothing right now.

* Any questions you might have
None.

Behzad Raeisifard

* What you’re currently working on, or have just completed
I am currently working on the design for compute_trophies to make sure that it can create trophies for new review requests as well as import trophies from previous review request.

* What you’re working on next
Implement the above mentioned design.

* What’s blocking you from progressing
Nothing right now.

* Any questions you might have
Do we want the trophies to be imported when a userpage is accessed or when an old review request is viewed?

Edward Lee

* What you’re currently working on, or have just completed
I’ve fixed the issues for r/4662 and some issues for r/4699. Currently working on the open issues for r/4699.

* What you’re working on next
Improving detection algorithm for r/4699

* What’s blocking you from progressing
I have a roadblock with with r/4699 that I need some help with (see question below).

* Any questions you might have
For r/4699, Summary and Description are used to determine a matching review request. If they are not provided and guessing is not enabled, we still want to guess them, but the values won’t be used in the update. The logic for guessing is currently embedded in the diff logic, and is a bit difficult to make independent. An easy hack is to enable guessing before calling get_diff(), and turn it off and restore the original summary and description values after extracting the guessed values. This is approach isn’t very clean so I’m not that fond of it. Any suggestions on how I should proceed?

Natasha Dalal

* What you’re currently working on, or have just completed
– Put up a WIP review request with a very basic backend function to get reviewers
– Working on front-end views to handle displaying and interacting with review suggestions.

* What you’re working on next
– Will be continuing front end work
– Will also be continuing to make the back-end function smarter.

* What’s blocking you from progressing
– Need to get through the django query documentation before moving on with the back end stuff.

* Any questions you might have
– None.

Mary Elaine Malit

* What you’re currently working on, or have just completed
I just finished adding the remove functionality for each checklist item. Also I fixed some of the formatting issues David pointed out in his review.

* What you’re working on next
I’m thinking of either starting on the back-end, or make more progress with the WIP, such as adding the edit functionality and minimized view.

* What’s blocking you from progressing
Nothing.

* Any questions you might have
Is it better to start on the back-end and API now, or should I continue to develop the prototype?

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/

How to Configure PyCharm for Review Board Development

So you’ve just spent a few hours going through the Getting Started guide and have finally gotten your development environment configured (and hopefully all the unit tests passed!). Getting the dev server up and running and seeing Review Board load up in your web browser for the first time is a tremendous accomplishment, and you should be proud to have gotten this far! But greater challenges lie ahead as you start hacking. If you’ve just started working on Review Board like me, you’ll want to take advantage of all the tools at your disposal to make the learning curve as painless and enjoyable as possible as you try to figure out how the pieces in the project fit together. For me, an IDE with a good GUI debugger is a must because it allows me to dynamically step through the program’s runtime execution, as opposed to having to figure it out statically through code inspection.

PyCharm is my IDE of choice. Compared to Eclipse with PyDev, I’ve found PyCharm to be much more friendly to set up, and it doesn’t make unnecessary modifications to your Git repository when you import the project, leaving you in a clean state before you start making changes to the code. PyCharm’s built-in support for Django is also a nice feature to have.

Configuring PyCharm with your Review Board project is simple (based on PyCharm 2.7 on OS X 10.8.5) :

  1. Open up PyCharm. Under the Quick Start pane, click Open Directory and select the top-level Review Board directory (the one that you cloned with Git). e.g. ~/reviewboard.
  2. Once the project is loaded, open the Project Settings window by going to PyCharm > Preferences.
  3. Under Project Interpreter > Python Interpreters, add the python interpreter you are using. If you’re using virtualenv, PyCharm is also smart enough to recognize it. You should see a list of python packages associated with the interpreter you have selected.Image
  4. Go to Django Support. Check Enable Django Support and set the Django project root directory to the top-level Review Board source directory. For example: /Users/Edward/reviewboard. Settings and Manage Script should be set to reviewboard/settings.py and reviewboard/manage.py, respectively.Image
  5. Save your changes and open the Run/Debug Configurations window by going to Run > Edit Configurations…
  6. Add a new Django server configuration and set Host to 127.0.0.1 and Port to 8080 (the default settings for Review Board’s development server when you run ./contrib/internal/devserver.py). Make sure Python interpreter is also set to the one you have previously configured.Image
  7. Click the Run or Debug button to start the development web server. Happy debugging!