I haven¹t looked at it because it is huge. But preliminary scan:
- there are unit tests missing for changes to core CS code
- uses com.amazonaws.util.json (why?)
- code format does not follow coding convention ( placement of {}, camel
case api_interface )
- package namespace is com.cloud instead of org.apache for new files
- no file-level comments. What does LocalAccountManager do? Why does it
exist? Etc.
- No interfaces, only abstract classes. No use of @Inject. While this
might be OK, it does make it harder to test and does not follow the rest
of ACS conventions.
I would urge the submitter to break up the submission.
A) the changes to CS core, with explanations / comments and tests
B) the sync service should be an interface with concrete implementations
in its own package
C) more tests
On 3/10/14, 3:37 AM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote:
>Hi everyody,
>
>The people at sungard have been making this change for 4.4 and I want
>to merge/apply it this week. It is more then a screenfull and might
>cause issue is a setup or two.
>
>have a look at https://reviews.apache.org/r/17790/
>
>a ref to ticket and fs page are in the review request.
>
>kind regards,
>--
>Daan
