wangsai / oppia

Automatically exported from code.google.com/p/oppia
Apache License 2.0
0 stars 0 forks source link

Code review request for gadget-visibility-validation branch. #771

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Branch name: gadget-visibility-validation

Link to the relevant commit(s): 
https://code.google.com/p/oppia/source/detail?r=481156a64843b65cec70505eb6d4300a
4054ebd4&name=gadget-visibility-validation

Purpose of code changes on this branch:
- Validates gadget size and fit considering visibility in each state.

When reviewing my code changes, please focus on:
- 
extensions.skins.skin_classes.GadgetPanelSpec._get_state_names_gadgets_are_visib
le is not DRY with 
core.domain.exp_domain.SkinInstance.get_state_names_required_by_gadgets. 
Considered converting the latter to a @staticmethod to run separately against 
each panel, but the 3-4 lines of code are simple enough it didn't seem worth 
introducing a cross-module function. What do you think?
- Broke up the new validation error in skin_classes.py lines 175-180 across two 
rows to stay within 80 char limit. This seemed preferable to a backslash, but 
is there a better way?

After the review, I'll merge this branch into: develop

Original issue reported on code.google.com by anu...@google.com on 21 May 2015 at 6:07

GoogleCodeExporter commented 9 years ago
Initial review round done. (Note that you don't need to file a follow-up review 
request -- I'll automatically get subsequent notifications when you reply to 
the comments and make a follow-up commit.)

Original comment by s...@seanlip.org on 21 May 2015 at 7:55