Author Archives: endee

Status Report: Nov 9, 2013

Natasha:
* What you’re currently working on, or have just completed
I updated my front-end code based on feedback from my first review request for it and posted an updated review request.

* What you’re working on next
I plan on switching focus to the back-end again this week to return a better/more accurate list of suggested reviewers

* What’s blocking you from progressing
Nothing

* Any questions you might have
None

Allisa:
* What you’re currently working on, or have just completed
I now have a test executable file uploading and downloading in the back-end. Also finished blog post.

* What you’re working on next
Polishing up the executable file test and moving on to the allowed hosts test.

* What’s blocking you from progressing
Nothing

* Any questions you might have
None

Elaine:

* What you’re currently working on, or have just completed
I am almost finished working on the web API resource, and should have a WIP request for that by tonight.

* What you’re working on next
Making checklist items editable.

* What’s blocking you from progressing.
The WebAPIResource is a pretty big class, and I’m still trying to figure out bits and pieces of how it works.

* Any questions you might have.
I’m a little confused about the links dictionary which maps a resource to its url, as well as the list_child_resources and
item_child_resources. I’m just not sure which ones I’m supposed to use.

Edward:
* What you’re currently working on, or have just completed
I’ve addressed the open issues in my review requests. I started looking into how search works in RB and read up on the basics of Haystack.

* What you’re working on next
Create a simple search prototype using Haystack and Whoosh.

* What’s blocking you from progressing
Nothing

* Any questions you might have
None

Behzad:
* What you’re currently working on, or have just completed:
The logic for assigning a trophy based on a review request is complete. I am working on testing it.

* What you’re working on next:
Connecting this to the UI or abstracting the whole logic as planned.

* What’s blocking you from progressing:
Having some issues with Unit testing.

* Any questions you might have:
It seems like the test is unable to find the compute_trophies function. I am working on figuring that out now.

Advertisements

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.

 

 

Blog Post — My Review Board Experiences Thus Far.

I’m Natasha Dalal,  A 4th year student at the University of Toronto. I’m working on Review Board as part of the UCOSP program. While I’m still in the starting stages of my work on Review Board, my experiences thus far have been incredibly positive.

We started off the semester by setting up our development environments and fixing some “easy fix” bugs listed on the review board bug tracker site. This was our entry point into the review board code-base. Setting up the development environment proved to be a bit trickier than I had expected, mostly because of dependency issues. However, there was a wealth of information available in the documentation section of the review board website, and the mentors were really helpful on IRC. After trying and failing to set-up my environment locally, I ended up setting it up using a virtual environment that one of the mentors had set up for us. This came with its own set of bugs and problems, but again, the mentors came to the rescue and were super responsive and available to help troubleshoot any issues as and when I faced them.

About 3 weeks in, the UCOSP students (5 of us) from across Canada, and all the mentors met in person for a 2 and a half day code-sprint at the Mozilla office in Toronto. It was instantly apparent, that we were the best team there (Biased, me? Not at all.) But seriously, the code sprint was a wonderful experience. We were in a room full of incredibly talented and motivated people working on some really interesting and diverse projects. Meeting the review board team in person, and being able to put a face to the IRC nicknames was awesome! Experiencing first hand how excited everyone was about Review Board was even more awesome! At the sprint, we got a chance to get to know each other a bit better, as well as a chance to learn more about the architecture and structure of Review Board. That weekend was a milestone for me, because I made my first ever open source commit, and even though it was just a tiny CSS change, it felt really good. By the end of the weekend, every student on the Review Board team, had successfully submitted their first patch to Review Board and earned their very own Review Board t-shirts for their accomplishments.

At the sprint, I discovered some useful tools to help me navigate my way around the project. While I started out with a stubborn desire to strictly use vim for all my code-editing needs, I quickly realized that I am nowhere near being the vim master I dream of being, and so, in the interest of time, I decided to switch to using Sublime. While I definitely haven’t explored the full potentials of Sublime yet, I did find some things in particular to be very handy. For one, I like that you can “open folders”, which gives you a side-bar in sublime through which to navigate through a collapsible tree structure of the sub-folders and files of the given folder. There is also a powerful search function across all files in the open folder. The code-completion and syntax highlighting are also pretty useful. Additionally, if you know the name  (or part of it) of the file you want to jump to, the Ctrl-P shortcut lets you quickly search for and open that file from within your project. There are also a ton of plug-ins available to help customize your sublime experience.

As someone who has never really programmed in python before, I also had the joys of learning about pdb, which is the python command-line debugger. It was incredibly easy to use, and turned out to be very helpful as I attempted to debug something (that wasn’t actually a bug) over the weekend. Setting a break-point in pdb is as easy as adding the line: “import pdb; pdb.set_trace()” to the part of your code that you want to debug. This then allows you to use the debug prompt in your shell to actually debug things. There is a blog post about this on the review board blogs at https://reviewboardstudents.wordpress.com/2012/01/22/the-python-debugger-pdb/.

Towards the end of day 2 of the sprint, we all sat down and discussed potential projects for the semester. I believe all of us ended up choosing from an existing list of student project ideas, although we were also given the liberty of coming up with our own. The project I chose was to implement a suggested Reviewers feature, which examines past activity to suggest prospective reviewers to a user posting a review request. I have a post outlining my basic strategy for it on hackpad, which I expect to evolve as I make progress on it. Here is a link to it: https://reviewboard.hackpad.com/Suggested-Reviewers-Feature-QI4YGdzLEWy .

So far, I have explored some of the areas of the code that I will need to look at, and have implemented a very simplistic function to work on the back end. As mentioned before, my experience with python and django is limited, but I have found it to be extremely powerful, easy, and convenient to work with. I find the organization of django projects and it’s use of MVC very appealing and easy to work with.

Taking part in UCOSP is one of the best decisions I have made for myself in a while. This is either a testament to how great I think the program is, or to how terrible I am at decision making, and I’m choosing to go with the former 🙂 . In a sea of mundane course-work, my review board project is what’s keeping me afloat. That was a little dramatic, I’m actually enjoying most of my classes, but this one slightly more than the others, and I expect the practical skills I gain from it to be exceedingly rewarding in the long run. On that note, I am grateful for the existence of UCOSP and for being able to participate in it. I am also really happy that I chose to work on review board, and hope to keep contributing to it after my UCOSP term is complete. The Review Board team is wonderful, helpful, and extremely dedicated, which makes them a pleasure to work with. I’m excited to make more progress on my project, and hope that the rest of the semester continues to go as smoothly as it began.

Meeting Minutes (September 15, 2013)

Opening Announcements:

Code Sprint is next weekend, work out travel and hotels ASAP if required and you haven’t already!

edwlee: 

  • Started on an EasyFix
  • Needs to review the patch he posted, will be working on it later today
  • Question: Wanted to know whether to merge/rebase from master before submitting an updated patch:
    Answer: yes. You can safely merge from master into project branch, or rebase… Some people find rebase scary.
  • Going to try finishing his current bug today and then will go through some backbone.js tutorials and maybe start a new bug.

elaineM:

  • fixed the bug where submitted starred reviews weren’t showing under ‘starred reviews’.
  • Question: While fixing the bug her starred_review_requests_count got out of sync. Is there a way to fix it?:
    Answer: ./reviewboard/manage.py fixreviewcounts –> resets all the counter caches.
  • Question: What does starred_review_requests.public() do?:
    Answer: public() returns only the starred_review_requests that are public facing (published, non-closed review requests). Starred_review_requests is defined in accounts/models.py, and is a ManyToMany to a series of ReviewRequests. Changing it to filter(public=True) will get published ones that may be up for review, or submitted, or discarded. The public flag/field in ReviewRequest indicates if it was ever made public.
  • She had forgotten the password to her admin account. Instructions on how to reset:
    For an admin account, manage.py changepassword will let you update the password. (ref: https://docs.djangoproject.com/en/1.4/topics/auth/#changing-passwords )
  • Also: ./reviewboard/manage.py –help : lists all useful manage.py commands!

endee:

  • Got her dev environment set up finally
  • picked an easy fix bug: The z-order of “filter” is above the drop-down “account” menu.
  • She got distracted by other school deadlines, but will start working on it tomorrow.

Tweek-web:

  • Having problems with dev environment set up: pysvn issues.
    Note: pysvn isn’t 100% necessary to get started on an EasyFix bug. It makes tests fail cuz it’s required for some tests, but RV still runs as long as not using an SVN repo.
  • edwlee’s solution to pysvn troubles: downloaded the precompiled pysvn binaries for osx and copied those into his virtualenv’s site-packages directory.

classy_baboon:

  • Set up dev environment and picked an EasyFix Bug.
  • He picked the bug where clicking enter didn’t search.
  • Had some troubles with his vm.
  • Decided to use PyCharm as an IDE.

OTHER NOTES:

How to GIT:

Any time you modify code, create a new branch or use one you already created (if it’s part of the same thing going up for review). There are no branches that already exist that you will commit code to!

Workflow: Pull from master –> create branch for your feature –> merge master in branch periodically .

NO PUSHING BRANCHES AT ALL THIS SEMESTER. Post patches to reveiws.reviewboard.org and someone else will push it after it has been reviewed. We can fork on github and push work in progress to that if we want, but then do not rebase any branch you pushed.

Encouraged Workflow Regarding submitting patches: post Work-in-progress patches on reviews.reviewboard.org. This helps to get feedback early. Convention: put WIP in review request title/summary e.g. “[WIP] Add the best feature ever” to indicate that it is not a complete feature.

IMPORTANT: Do not commit to master, or merge into master. MASTER IS UNTOUCHABLE. If you accidentally do it, talk to someone about it. master reflects the current upstream in-development branch of RB. once your commit lands, you can blow your feature branch away, since the change will be pushed to master. When a commit is in master, it is in the main codebase.

 

Review Bot: (Automated review bot that will comment on some styling stuff!)
It runs static analysis tools, which can be added as plugins. Right now on r.rb.org it runs PEP8 and pyflakes on your code. There are plugins for CPPlint, Buildbot, CPPCheck, and JSLint right now as well, but we don’t run them

 

Hackpad:
instead of wiki, we will use hackpad.com for all our wiki-ish needs. It is a mixture of Google docs + a wiki.
http://reviewboard.hackpad.com

To Do: Set up an account.

 

Some Useful Pages:

HackPad Student Tips and Tricks

Blog Posts Written by Previous Students (More Tips and Tricks)

Project Idea Page (We are welcome to add to it).