Launchpad itself

Code review comment for lp://staging/~danilo/launchpad/bug-772754-other-subscribers-sections

  1. bug-772754-other-subscribers-sections
  2. Merge into devel
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Danilo,

I really like your approach to JS development and enjoy learning from your branches.

Things like the CSS_CLASSES dict are a nice touch to encapsulate info in a DRY fashion.

* Y.Node.create('<div></div>') can be written as Y.Node.create('<div />') and many find it more readable. YUI does the right thing, so you can even use it on elements that aren't really allowed to be self-closing.

* Defining 'level' at first use, maybe in getOrCreateSection, would be helpful.

I see you use a lot of bare comparisons against null and undefined whereas others seem to prefer to use the Y.Lang tests. Are they equivalent? Perhaps this is an issue we should decide on and put in our (as yet mythical) JS Guidelines. Your thoughts are appreciated.

Thanks for catching the feature flag that I think should've already been removed.

review: Approve (approve)

« Back to merge proposal