exo-addons / cloud-drive-extension

eXo Cloud Drive extension
10 stars 14 forks source link

Review pull request for upgrade plf43 #49

Closed quynhntn closed 9 years ago

quynhntn commented 9 years ago

https://github.com/exo-addons/cloud-drive-extension/pull/48

pnedonosko commented 9 years ago

What actual changes about PLF 4.3 in the pull-request? Do we have UI difference with 4.2 or changes in container components? FYI version is wrong it will be 1.4.x for Platform 4.3. Use of infinispan configurations are only for tests - thus has no impact on the add-on functionality.

quynhntn commented 9 years ago

Hi Peter,

For the version, it's just the snapshot version for my branch, you can change the version when merging into develop or other branches, I don't have permission on that.

For the impact, yes, it does not impact on the add-on functionality. When upgrade to new version of plf, there are some UT ran fail due to the Cachable... class does not exist anymore. I changed some configuration to make those passed.

Thanks,

Quynhnn.

On Mon, Sep 21, 2015 at 4:23 PM, Peter Nedonosko notifications@github.com wrote:

What actual changes about PLF 4.3 in the pull-request? Do we have UI difference with 4.2 or changes in container components? FYI version is wrong it will be 1.4.x for Platform 4.3. Use of infinispan configurations are only for tests - thus has no impact on the add-on functionality.

— Reply to this email directly or view it on GitHub https://github.com/exo-addons/cloud-drive-extension/issues/49#issuecomment-141922498 .

pnedonosko commented 9 years ago

Ok for tests and versions, I'll apply this proposal. But tell me if it is critical for eXo Cloud if nothing impacted functionally. Otherwise you can use latest stable version 1.3.0 there. There are no plans for a version 1.4 (for PLF 4.3) in near future.

quynhntn commented 9 years ago

Hi Peter,

This morning, I tried with 1.3.0 and 1.3.0-RC1. It builds success but it throws exception when I launch UXP with cloud. It said:

Caused by: java.lang.NoSuchMethodError: org.exoplatform.container.xml.ValuesParam.getValues()Ljava/util/ArrayList; at org.exoplatform.clouddrive.ecms.CloudDriveUIExtension.(CloudDriveUIExtension.java:53) ~[exo-clouddrive-services-ecms-1.3.0.jar:1.3.0]

And yes, the UXP upgrade to plf43 and kernel-cloud 2.5.x-UXP-SNAPSHOT. The above method does not exist anymore. It is "List getValues()"

Of course, it works fine with my branch, UXP can launch success with exo-cloud.

Please take a look at.

Thanks,

Quynhnn.

On Mon, Sep 21, 2015 at 8:15 PM, Peter Nedonosko notifications@github.com wrote:

Ok for tests and versions, I'll apply this proposal. But tell me if it is critical for eXo Cloud if nothing impacted functionally. Otherwise you can use latest stable version 1.3.0 there.

— Reply to this email directly or view it on GitHub https://github.com/exo-addons/cloud-drive-extension/issues/49#issuecomment-141971066 .

pnedonosko commented 9 years ago

Well I see the problem with the method - indeed it doesn't fixed in the pull request. Also I don't understand how it works on your branch, have you added that method back there or changed the add-on code?

quynhntn commented 9 years ago

Hi Peter,

For the problem, the return result is List instead of ArrayList but the treatment is the same as it is so it does not need any fixes, it just needs to upgrade version of platform and JCR to make sure it does not have any conflict.

Thanks,

Qunhnn.

On Tue, Sep 22, 2015 at 2:17 PM, Peter Nedonosko notifications@github.com wrote:

Well I see the problem with the method - indeed it doesn't fixed in the pull request. Also I don't understand how it works on your branch, have you added that method back there?

— Reply to this email directly or view it on GitHub https://github.com/exo-addons/cloud-drive-extension/issues/49#issuecomment-142200388 .

pnedonosko commented 9 years ago

OK. Thank you for the study. I'll do an upgrade to PLF 4.3 and plan the release 1.4.