[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-158015787 Ping @DaanHoogland @miguelaferreira @karuturi @borisroman @bhaisaab @remibergsma More test results, guys! It looks good to me... but my vote

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread borisroman
Github user borisroman commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-158017282 @wilderrodrigues LGTM :+1: Always in support of removing old/dead code! :) Testing: CentOS 7.1 Setup Managment/Hypervisor ```

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread borisroman
Github user borisroman commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-158020779 @jayapalu That's we're git's versioning system comes into play. We want to keep the current working directory as clean as possible. Meaning, without unused/dead

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread jayapalu
Github user jayapalu commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-158020225 @wilderrodrigues @karuturi VR scripts got stabilised over years. If any thing wrong in new implementation there is chance to look into the VR scripts and

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157994184 Thanks, @karuturi! I'm testing against XenServer 6.2 now. Given the changes on the HyperV Resource class, I'm not expecting problem there. The

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-158049050 @jayapalu, I'm with @borisroman on that one. We are no longer using shared file systems to keep files stored. We have a good version control system and the

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread remibergsma
Github user remibergsma commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-158117634 I also agree we shouldn't keep unused files in the active tree. We already got some PRs against files that were not in use and that is a waste of time for

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-19 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1084 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157785310 Ping @DaanHoogland @remibergsma @miguelaferreira @bhaisaab @karuturi @borisroman First round of tests done and got the same results! I will run

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157810368 @wilderrodrigues maybe we can ask @anshul1886 to run the test wuite on hyperv? --- If your project is set up for it, you can reply to this email and have

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157818838 If @anshul1886 can test it, just to make sure all is fine, would be very nice! @karuturi, do you have a HyperV test environment? :)

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157859746 More test results: * Environment - Management Server on CentOS 7.1 - One KVM Host on CentOS 7.1 - Agent + Common RPMs built from

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread miguelaferreira
Github user miguelaferreira commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1084#discussion_r45184075 --- Diff: utils/src/main/java/com/cloud/utils/StringUtils.java --- @@ -310,6 +310,11 @@ public static String mapToString(final Map

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157705996 good points, I do want to have a more generic way of creating complex log messages though. might be useful to construct womething like this for it anyway.

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157693480 @miguelaferreira discussed with me the over-engineering wrapper method just to add 1 test and we agreed on getting rid of it for the following reasons:

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1084#discussion_r45176128 --- Diff: utils/src/main/java/com/cloud/utils/StringUtils.java --- @@ -310,6 +310,11 @@ public static String mapToString(final Map

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157654321 less is more! reviewed java code only, didn't check the removed scripts. LGTM --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-18 Thread karuturi
Github user karuturi commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157945578 @wilderrodrigues nope. No HyperV environment. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-17 Thread wilderrodrigues
GitHub user wilderrodrigues opened a pull request: https://github.com/apache/cloudstack/pull/1084 CLOUDSTACK-9067 - As I developer I want to remove all the unused router-shell scripts from ACS This PR removes the unused shell scripts that were present in the ACS project. Those

[GitHub] cloudstack pull request: CLOUDSTACK-9067 - As I developer I want t...

2015-11-17 Thread wilderrodrigues
Github user wilderrodrigues commented on the pull request: https://github.com/apache/cloudstack/pull/1084#issuecomment-157625899 Ping @remibergsma @miguelaferreira @karuturi @DaanHoogland @borisroman Partial tests results, but I will still run some tests against XenServer 6.2