Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161775836
@remibergsma awesome :)
---
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 asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1134
---
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 is
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161750205
@pdube indeed, I waited because of the comments but that has been addressed
now :-)
---
If your project is set up for it, you can reply to this email and have
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161657343
Hey guys, there are enough LGTMs, can we get this merged?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as we
Github user ustcweizhou commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46554416
--- Diff:
server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java ---
@@ -229,59 +205,99 @@ public AffinityGroupVO
doInTransaction(T
Github user pdube commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46553780
--- Diff:
server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java ---
@@ -229,59 +205,99 @@ public AffinityGroupVO
doInTransaction(Transac
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161560287
LGTM based on these tests:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a
tags=advanced,required_hardware=true \
component/test_
Github user ustcweizhou commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46519549
--- Diff:
server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java ---
@@ -229,59 +205,99 @@ public AffinityGroupVO
doInTransaction(T
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161537378
LGTM. tested
(1) create affinity group in project
(2) deploy vm with affinity group
(3) update vm affinity group
(4) delete affinity group.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161431929
@pdube will run some integration tests against this branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161286193
Code LGTM, not yet tested though.
---
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
Github user pdion891 commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-161004341
LGTM, tested in a home brew 4.6. AG work in project context.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHu
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160990441
@pdube good timing, testing it right now. expect feedback today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHu
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160978139
@resmo @ustcweizhou @remibergsma Any news on testing? LGTY?
---
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 the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160925744
Ok, let's discuss sometime outside the scope of this PR. and Let's Get This
Merged.
---
If your project is set up for it, you can reply to this email and have
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160638862
@DaanHoogland Ok, I understand what you mean. I do think that we need some
clarifications on what the standards are (search vs list, service vs query vs
dao, etc.) PS
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160532869
Yes, I understand and it makes sense to have that documented. It doesn't
have to mean we phase out QueryService. This makes sense to be there for people
with r
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160447903
@DaanHoogland I was talking about the implementation of APIs. Example,
let's say you create a new module, call it Exampe. So we can list, create and
delete Example. W
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160444699
@pdube, no, there is a page on cleaning up the API but as the semantic
version scheme commands we should only do that cleaning up per version 5.0. see
https://
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160441642
@DaanHoogland I did not find a guide to do it. I looked at the patterns I
found in other modules, and imitated those. I found it confusing because there
were a few di
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160417989
Did a quick review and it looks great. One general remark; Let's keep
interface as orthogonal as possible, i.e. use 'list' as the prefix for search
functions a
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46092380
--- Diff: server/src/com/cloud/user/AccountManagerImpl.java ---
@@ -2406,98 +2406,10 @@ public void buildACLSearchCriteria(SearchCriteria sc
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46092351
--- Diff: api/src/org/apache/cloudstack/query/QueryService.java ---
@@ -131,9 +132,7 @@
ListResponse listIsos(ListIsosCmd cmd);
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46092314
--- Diff: api/src/org/apache/cloudstack/affinity/AffinityGroupService.java
---
@@ -18,47 +18,36 @@
import java.util.List;
+impo
Github user remibergsma commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46088753
--- Diff: test/integration/component/test_affinity_groups_projects.py ---
@@ -0,0 +1,1083 @@
+#!/usr/bin/env python
+# Licensed to the Apache
Github user pdube commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46086641
--- Diff: test/integration/component/test_affinity_groups_projects.py ---
@@ -0,0 +1,1083 @@
+#!/usr/bin/env python
+# Licensed to the Apache Softw
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160288372
@pdube Awesome you added all the tests! Thanks! Will have a run some tests
myself soon.
---
If your project is set up for it, you can reply to this email and h
Github user remibergsma commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1134#discussion_r46079415
--- Diff: test/integration/component/test_affinity_groups_projects.py ---
@@ -0,0 +1,1083 @@
+#!/usr/bin/env python
+# Licensed to the Apache
Github user ustcweizhou commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160201982
This PR includes the unit test and integration test, very nice!
I will test it.
---
If your project is set up for it, you can reply to this email and have y
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/1134#issuecomment-160200278
awesome job! can not wait to get it tested.
If all is fine, it would be great if it gets into 4.5.3 (/cc @bhaisaab) and
4.6.1 (/cc @remibergsma). I know a few
30 matches
Mail list logo
