- If you guys have comments from us on reviews, and have fixed those comments, make sure to click Fixed on the issues (and make sure each one is actually Fixed, if you haven’t done so already)
- The review request object has a starred attribute, though reviews don’t, and i was wondering if that is something you would want to add, or if the star would refer to the review request that the review was made for?
- I think adding it for reviews would just complicate things. So, I don’t really see a point myself to being able to star reviews… it *might* be useful for a review requester to star reviews they need to address, but I think that’s just adding extra complexity.
- Can I just scrap the star column then?
For the frontend, should the reviews and review request tabs always be shown independent of the # of reviews someone has made?
- I’d say yes. Better to be consistent.
- I have spent this week implementing the configuration UI. Things are already working besides dynamically adding new rules, but I know how to do that, just have not had time to do that.
- Can you add a screenshot of it to your review request? Early screenshots are fine too 😀
- There are still a number of open issues on my review request. I have not fixed them yet, those files are still under development and change frequently. I have been planning to first implement all features and then polish things.
- I worked on fixing my code and started the front end for adding widgets. I saw that David reviewed my code, and I will get to that soon.
- I will post the code I used to make my screenshots so the mentors can take a look at my approach.
- For screenshots on a retina display, I recommend using a tool like RetinaCapture. The high-dpi screenshots are very hard to review. Or at the very least, scaling down the screenshots by 50%. Actually I guess with this being Android, scaling down might be your only option. Can you post that code ASAP please?
- I would like some comments on the design.
- I think the design works.
- I didn’t post the code so far because it was so bad and all over the place, but I will do it.
- That’s perfectly fine, we still would like to see it, and we can comment on general things like the approach rather than code cleanliness.
- Have you thought about the next step you’re going to take after this login form?
- Not really.
- The css now effects the registration form also so have to do testing with that also.
- That might be something to investigate while you wait for feedback on this stuff. Now that you’ve got the media queries setup etc you might want to consider styling a few other things as well.
- [Excused – CMU on Spring Break]
- I’ve been working on the API for adding repositories. Testing for that is interesting… quite overwhelming. I don’t think I have questions I could ask now, I’m still exploring the code base, wrapping my head around things.
- I have a question about closing a review request: so review requests can be closed generally, without a user to close it?
View code can do anything. The issue here is that _find_review_request includes some permissions checks, so I think what we should do is split that up into two methods. One of those can just fetch the ReviewRequest object based on the params, and the other can use that and then add the checks.
It’d be nice to make that a decorator too.
Keep the views simple and let them take a parameter after fetching and after checks have been done assuming those would be useful to the new view.
- Just to clarify, I should modify _find_review_request itself? Or create my own version of it where the two parts are split?
I think you should take the actual finding part out and move it into a new method. So modify it.
- Leave _find_review_request as the one that does the permissions checks, and it can call your new thing. That way you don’t have to update tons of code.
- [Excused – CMU on break]
I’ve made part 1 of my project, so the code line mumbers work now as a links, when you hover over those. Part 1 is making the links for the line numbers, and part 2 is to highlight the code line where the url is linked. I’ll post a picture of the code links after the meeting.
- I will update the old review request.
So I’ve been working on the diff expansion and trying to get more familiar with how it works. I didn’t get as much time this week as I had hoped to work on it, but I should be more free this coming week
- Resource interpreted as Script but transferred with MIME type text/html: “http://localhost:8080/r/2/fragments/diff-comments/2,1,3/?queue=diff_fragments&container_prefix=comment_container&1394390353″ can be safely ignored.
- I’d like to request screenshots on the review requests if there’s anything to show.
- I’d recommend going through the change and making sure the coding style matches the style we use throughout the rest of the codebase. Self-review is an important thing to practice. Go through the patch you have up, make any improvements you can think of, make sure all the styles (line lengths, var statements, braces, etc.) all match how we do things. Optimize the code best you can, and get it ready for us to try to give a final pass.
- It’s generally a good idea to be on IRC whenever you’re working on this. If you get stuck ask questions often and early. For the most part, there is *usually* a mentor around. Early morning PST not included.