Meeting Log November 24, 2013
Mike: If your stuff isn’t out of WIP, it should be really close to being out of WIP! Soft pencils down is *next weekend*, according to the calendar! You’re going to get out of WIP sometime in the next few days so that we have a chance to go over your patches with a fine tooth comb.
I basically just have to set it up so it can do more filetype checks cleanly. Like have a list of them rather than just doing the one type. That shouldn’t take long. I am a little concerned about the other check though. I didn’t get as far as I’d have liked this week and have not had the chance to start it. Once I’m on it it should be ok I’m just not entirely sure what’s involved in it.
David: the ALLOWED_HOSTS one? it’s really simple. Import django’s settings, and look at the value of settings.ALLOWED_HOSTS. If it’s set to [‘*’], show a warning
Allisa: So basically as long as it’s not * or blank, it’s good? Are there any particular values that would be troublesome?
David: If it’s set to the wrong thing then they won’t be able to access the site at all.
Allisa: And along with making it a list, I need to send more valuable data to the template. Like make it more than just success/failure.
Mike: And does this look like something that’s attainable within the timeframe? (including time for review)
Tweek: I think so… I won’t have much time to work on it today/tomorrow but should be able to spend a lot of tuesday/wednesday/thursday on it.
So I wanted to have a non WIP review request up today, but I got a bit stuck on something that I was talking to ChipX86 about last night. I plan to spend all day tomorrow on this though and hopefully have it up by tomorrow night. I added the vertical space to the front-end. it looks a lot better. I can screen shot if if you like. http://imgur.com/vO1dUJY
I’m close to connecting the UI to the backend, at least for the review request page. i should have a non WIP review request up in a few days. Right now, the received_date is being set in the model itself and TrophyManager knows nothing of it. I was thinking that TrophyManager could set the received_date itself from timezone.now and could display it (when calculating for the first time). So it doesnt have to get it again from the model.
Christian: TrophyManager shouldn’t concert itself with defaults for fields, if the model can easily do it. TrophyManager’s job is to fetch, create and return model instances. The model instances is what other code should mostly concern itslef with, when wanting to know details about a trophy.
Mike: generally, in model-view-controller / model-view-template, a guiding principle is “fat model, thin controller”. Meaning, if the logic can go into the model, it should probably go in the model.
Behzad: it was previously mentioned that on the user page, there should be a row of trophies with the id and received date displayed under the icon. Does the id refer to the primary key for the trophy? What if multiple trophies are given out for the same review request?
Christian: review request ID. That’s fine, each one is an individual trophy
Behzad: Okay the title and description mentioned on the hackpad article for trophy case design mentions a title and description for the TrophyType. The title would be something like “Palindrome Trophy” right? What would the description be?
Mike: I believe that’d be a description of what was required in order to receive that trophy. “You created a review request with an ID that is the same forwards and backwards”, something like that.
Christian: I don’t remember these parts of the spec, but on a review request, the text taht shoud be displayed is what’s displayed today “<user> got review request #<id!>!”, and the TrophyType should be responsible for providing that text. so that a trophy later can say something like, “You got this because you’re a great guy!”
Behzad: okay, so the display text does not need a field in the model?
Behzad: But the description would, righ?
Christian: nope. that’s going to be something provided by the TrophyType the model should store the raw, essential data. The TrophyType is the interpreter of that data, when necessarythe TrophyType is the interpreter of that data, when necessary
Mike: since the description will never change from trophy to trophy, that’s why it’s stored in the type.
Behzad: oh i thought it would change trophy to trophy. You created a review request with an ID that is the same forwards and backwards is only for palindrome. for milestone it would be different
Mike: ok, that’s absolutely correct
Behzad: okay, so for the same trophy its the same
Mike: but for multiple palindrome trophies, the description is the same
Christian: 1) we should never tell the user this. It gets rid of the fun so the text we have today is what we should show them for these two trophies. 2) as m_conley said, trophies of the same type are likely going to have the same description if they don’t, that’s because the TrophyType decided it was going to be different for that particular trophy. By having it in TrophyType, it can do things like localization, too.
David: I think for palindromes we should just say “You got a fish trophy!”
Behzad: I have one more question. right now I have this: http://pastie.org/8505815. It works for these two, but we’ll probably want to do it in a non-procedural way.
Christian: yeah, you need to use the registration design we discussed
Mike: a dict of names to classes might be useful there
Christian: you should do almost exactly like the review UIs
Behzad: okay, i haven’t done that yet. would i also need an unregistration system?
Mike: yeah, for when extensions are disabled
Christian: yes. Copy the review UI pattern and you’ll get all that
Behzad: one more question. if i were to display 10 trophies maximum lets say, on the user page for someone, should a link take them to another page that displays the rest of them, or perhaps an expand button?
Mike: at this point, trophy assignment is rather rare
David: and expand button might be nice but I think there’s probably enough stuff on your plate right now
Christian: yeah, focus on the abstraction and display on the review request page for now
Elaine: here’s a link to my github: https://github.com/manderelee/rb_checklist
Mike: so, you’re hitting a strange bug where the checklist is being rendered multiple times, and multiple POSTs are going out?
Elaine: Yeah… but I have a speculation about where I might be going wrong. I haven’t tried yet… but in my template, I just added a script that creates a new checklist right away. I think I might have to add something like document.ready or another one of those to control when it actually gets created. Just a speculation.
Christian: it does sound like a view might be applied more than once
Elaine: So I know that it’ll be nice if I could review other students’ code… but since I’m not familiar with their stuff, what kind of things would be helpful to point out anyway?
Christian: code style is a good and easy one. You can also check comments and doc strings to make sure they make sense. sometimes it helps to just mentally trace through their code, see if anything stands out as being wrong, or see if there’s a corner case you can think of that may break it
Mike: you’ve got a good understanding of Backbone and WebAPIResource’s now, so if you see strangeness in those areas, don’t be shy to bring it up. don’t be afraid to just point out something that you’re curious about too because sometimes that’s indicative of a bug or a lack of documentation. At the very least, you find out what it does, and you learn. If y’all haven’t already noticed, you can filter out the student projects by looking at the submissions to the students group: https://reviews.reviewboard.org/groups/students/?show_submitted=0
and if your review request isn’t in there, add the students group to your reviewers list
I’ve got quite a bit done this week and i’m pretty happy with the results. Indexed search is fully working. All my latest stuff is up for reviews.
Mike: I know you were poking at query parsers last week – any movement on that front?
Edward: I got field searching to work as well without having to use an external query parser
but that depends on Whoosh’s query syntax
Christian: I think that’s fine if it doesn’t have all the features from PyLucene
Edward: which is quite similar to what Lucene supports: http://pythonhosted.org/Whoosh/querylang.html. The only downside is that we can’t easily swap the search back end with another one
Mike: I think that’s OK for now
Edward: but if we only want to support Whoosh then it’s not a big deal
Christian: realistically, I don’t imagine anyone would but maybe us (if we do something special for RBCommons)
David: I think the only issue is if there are multiple front-end servers
Christian: that’s probably more for us to figure out anyway
Edward: can you elaborate on that?
David: for large installations, they might split it up into a database server and multiple web servers but that means creating and maintaining the index file on each web server we can solve that in the power pack, though 🙂
Edward: but as long as whoosh is used then it wouldn’t be too difficult to implement, i’m assuming how search works now is Haystack just passes the raw query to Whoosh changing the default boolean operator to OR does nothing for Whoosh
Mike: did you file it with them?
Edward: Not yet
Mike: for mega karma, file it, and supply a patch. 😀
Edward: haha yeah i think i’ll do that this week