Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-211359351
@swill thanks, I was expecting for this one a long time
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1445
---
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 user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209024302
I did nto noticed that @swill LGTM.
So, it is OK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r59422694
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
-import
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209028959
I appreciate it. :+1:
---
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
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209025189
@pdube sorry, my brain is into something else right now and I didn't review
the scope of the code. Yes, your tests are valid tests for this change. So
yes, I will
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209026629
Just trying to help get things merged :)
---
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
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209023846
@rafaelweingartner @swill The only thing changed was the unit tests, which
I ran. And the only code change he did was to remove useless variables. @swill
also gave
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209021841
I would say it is missing an LGTM.
But, it is very good the code.
---
If your project is set up for it, you can reply to this email and have your
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209020807
@pdube we need to run CI against it first, but otherwise, yes it is ready...
---
If your project is set up for it, you can reply to this email and have your
reply
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209017286
@swill this looks ready, no?
---
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
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-199858341
Built and ran the unit tests. LGTM
[INFO] Apache CloudStack Developer Tools - Checkstyle Configuration
SUCCESS [ 1.106 s]
[INFO] Apache CloudStack
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-199535142
This LGTM. @pdube, would you mind reviewing this for me?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r56813778
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
-import
Github user alexandrelimassantana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r56808020
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r56799125
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
-import
GitHub user alexandrelimassantana opened a pull request:
https://github.com/apache/cloudstack/pull/1445
Fixed Profiler's unit tests bugs.
Problem:
The TestProfiler class was using Java Thread methods to test the
Profiler's functionality. That was causing the tests to fail
17 matches
Mail list logo
