On 12/1/2011 10:45 AM, Adam Young wrote:
On 12/01/2011 10:02 AM, Petr Vobornik wrote:
1) Should widget expect that array of values like ['value'] will be
always passed to update(values) method or the update method should
also work with simple values like 'value'?


The widget tests are higly subjective. Do not be afraid to change them,
especially if the assumptions they are written against have changed.
They were written more as a way to exercise the widgets than a
codification of behavior.

The best probably would be to support both, but the code would have to check the param type, which would make it more complex. If we're just going accept 1 type we should take the array because it can accommodate single value too. Some single-valued attributes are returned as arrays too.

2) Should widget load its own metadata - based on entity and its name
or it is responsibility of field (current implementation)?

Personally, I'd prefer if the Metadata was passed to the constructor, as
that allows us to compose it in multiple ways. That will be useful in
the future. INfact, I would prefer it if, instead of the Metatdata as a
chunk, we passed the indivual pieces that are required (pattern
specifically) so that how to build the object can differ, but I wanted
to hold off on doing anything like this until we had at least one more
specific project to integrate with. GO with the most striaght forward
solution for now.

Yeah, the metadata should be supplied by the field builder into the field's constructor. Then the field can use it to set the labels in the widget.

3) Should custom dirty checks, or other boolean checks (validate)
always return pure boolean value, or is integer also acceptable. Usage
of &= or |= operators is producing integer values because it actually
means 'foo = foo & bar' not 'foo = foo && bar'

Is there really a difference in Javascript? I so long as they evaluate
to true/false, it is probably fine, but is there the possibility for
confusion?

I think we should make sure we use boolean operator instead of bitwise. I'm not sure if we're actually affected, but there's a difference:

 1 && 2 => true
 1 & 2 => 0

Also all jsl warnings are fixed and jsl.conf for tests is added.

There's a whitespace warning in patch #49.

--
Endi S. Dewata

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to