On 11/04/2010 11:42 AM, Endi Sukma Dewata wrote:
Hi,

Please take a look at the new patch (also attached):

https://fedorahosted.org/reviewboard/r/99/

On 11/3/2010 1:59 PM, Adam Young wrote:
Very cool, but suggest we change the term. Would layout perhaps be better?

Renamed that to layout.

add.js line 34: Do we really need accesor like this? There is nothing
wrong with doing modifying the member directly. I see the code at line
62 that delegates it down the tree...I think there is a more
javascript-y way to do this. Look up Javascript accessors.

Now it's using setter/getter for entity_name.

If you are going to change a function header like on associate line
133, go ahead and remove the camel_casing as well. (manyObjPKey) as
you seem to be doing variable cleanup elsewhere.

Line 297, executor takes 7 params, that are all member variables of
"that". Since that.execute is invoked as a method, you can remove
these parameters and instead, internal to executor, refer to them via
this.<param>

Yeah. PLus, with the Bulk plugin, we'll want to change the name of the
bulk associator to something more correct, like single_call versus
bulk_call, and change the serial associator to use the bulk plugin.

I cleaned up the associators. I added a base class, I also combined the adder & deleter (both for serial & bulk) because once the parameters are cleaned up they are actually exactly the same code. We can rename these classes again later if necessary.

Typo line 344: that.member_attrribute

Fixed.

Also: remove the buttons for features that we are not going to implement
this time around
from the top of the page: Troubleshoot, Cull Disabled Rules, And the
TEst Rule link under quick links
You can leave Login SVC and Login Svc Groups , those are coming next,
correct?

They are commented out for now, will be added back as we implement them.

Add rule has a rule type field, but no guidance what to fill it in with. I suspect this should be a select. Without knowing what to put in here,
you can't add a rule. At a minimum, lets put in text 'allow or deny'

Fixed. Will open a new ticket for the drop down list.

Thanks!

ACK and pushed to master

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

Reply via email to