|
SaveModeType
- should be in the form.field.definition package
MultiProperty
- what would it take to make MultiProperty able to support more types than strings?
- field delegate should be renamed to handler
- unnecessary cast in setValue()
MultiValueHandler
- javadoc is confusing, should say that the responsibility of the handler is to store the value in a adapter
MultiValuesHandler
- extends SingleValueHandler, would it not make more sense to add an Abstract base class if all we need to share is a util method?
- the name is confusing would make more sense if this was MultiValueHandler
SubNodesValueHandler
- the terms root and parent seems to mean the same thing, can we use parent consistently instead?
- the term rootChild should be just child
- the name child sometimes seems to be the node holding the entries and sometimes the entry itself, we should use the term entry to make it clear which is what
- should not call applyChanges() since this should only be done once when actually saving the data, use getJcrItem() instead
- spelling error in SubNodesValueHandlerTest chield should be child
- unnecessary casts in getOrCreateChildNode()
- StringUtils.rightPad does not cut the string down to 20 chars, only makes it at least 20 chars long
MultiValuesHandlerTest
- copyright header says 2012
- should call applyChanges() only once per test
SingleValueHandlerTest
- copyright header says 2012
- should call applyChanges() only once per test
SubNodesValueHandlerTest
- copyright header says 2012
- should call applyChanges() only once per test
|
|
SaveModeType
- should be in the form.field.definition package
(ehe: current package package info.magnolia.ui.form.field.definition)
MultiProperty
- what would it take to make MultiProperty able to support more types than strings?
(ehe: I wanted to handle it in a more generic task MGNLUI-1455)
- field delegate should be renamed to handler (ehe:ok)
- unnecessary cast in setValue() (ehe:ok)
MultiValueHandler
- javadoc is confusing, should say that the responsibility of the handler is to store the value in a adapter
(ehe:ok)
MultiValuesHandler
- extends SingleValueHandler, would it not make more sense to add an Abstract base class if all we need to share is a util method?
(ehe: make sens, I was asking my self the same question.)
- the name is confusing would make more sense if this was MultiValueHandler
(ehe: MultiValueHandler is already the name of the implemented interface.)
SubNodesValueHandler
- the terms root and parent seems to mean the same thing, can we use parent consistently instead?
- the term rootChild should be just child
- the name child sometimes seems to be the node holding the entries and sometimes the entry itself, we should use the term entry to make it clear which is what
(ehe: ok, tried to add consistency to the variable name.)
- should not call applyChanges() since this should only be done once when actually saving the data, use getJcrItem() instead
(ehe: agree event un-necessary method, removed)
- spelling error in SubNodesValueHandlerTest chield should be child
- unnecessary casts in getOrCreateChildNode()
(ehe: agree)
- StringUtils.rightPad does not cut the string down to 20 chars, only makes it at least 20 chars long
(ehe: agree, my mistake, Name may be less than 20 char but not longer)
MultiValuesHandlerTest
- copyright header says 2012
- should call applyChanges() only once per test
(ehe: agree)
SingleValueHandlerTest
- copyright header says 2012
- should call applyChanges() only once per test
(ehe: agree)
SubNodesValueHandlerTest
- copyright header says 2012
- should call applyChanges() only once per test
(ehe: agree)
|