Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-219169448
Ok, what I will do is merge it into 4.6 and forward merge it to 4.7 and
then run CI on it there. I also need to verify what is forward merged from 4.6
because I don'
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-219169003
That is fine, I don't mind doing that. Is that the process here? This is
the first time I have handled a 4.6 PR. Will a 4.6 ever be released, or is
that just not i
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-219165095
@swill why not just test against 4.6 and then merge forward? if not, we
need to rebase the branch. Pretty sure @ustcweizhou already did that in-house.
---
If
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-219134524
@rhtyd I can run CI on this, but it is based against 4.6. I am not sure
what to do about that. Should @ustcweizhou at least rebase against 4.7 so we
can get it in a
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-218959585
@swill this can be CI tested and 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 projec
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-216208175
@ustcweizhou can you rebase against latest master and share state of your
PR, thanks
LGTM, though a marvin smoke test that can run with Travis would be great
Github user alexandrelimassantana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1212#discussion_r55956795
--- Diff: server/src/com/cloud/server/ManagementServerImpl.java ---
@@ -3567,7 +3567,17 @@ public boolean deleteSSHKeyPair(final
DeleteSSHK
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-175687426
LGTM, if we don't have any outstanding objections -- let's merge this?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-170342555
@ustcweizhou any update?
---
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 n
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164201602
@ustcweizhou @borisroman we will release both 4.6.2 and 4.7.0 RDs tomorrow
---
If your project is set up for it, you can reply to this email and have your
repl
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164199618
@ustcweizhou Thanks! Will see them when they arrive!
---
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 the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164198697
@DaanHoogland I will add some tests in the file when I have time.
currently I have some other tasks to do (not porting).
---
If your project is set up for i
Github user ustcweizhou commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1212#discussion_r47437500
--- Diff: server/test/com/cloud/user/AccountManagerImplTest.java ---
@@ -185,6 +186,8 @@
@Mock
GlobalLoadBalancerRuleDao _gslbRuleDa
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164191853
It is certainly not wanted but as it is in an otherwise working unit test
it is allowed. It does not interfere with the correct working of the system.
---
If
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164189121
Hi @DaanHoogland, well then that's your opinion. Mine is, as a developer I
do not want dead code to be added to the project.
Therefore I stand by my :-1:
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164188525
@borisroman I don't agree with your :-1: The artifact from a port in a test
class that otherwise compiles is worth a -1?
---
If your project is set up for it,
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164178288
Until either a test is added, which is preferred! Or the dead code (see
comment) is removed, :-1:
---
If your project is set up for it, you can reply to this e
Github user borisroman commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1212#discussion_r47435698
--- Diff: server/test/com/cloud/user/AccountManagerImplTest.java ---
@@ -185,6 +186,8 @@
@Mock
GlobalLoadBalancerRuleDao _gslbRuleDao
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-164174462
LGTM based on these tests:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a
tags=advanced,required_hardware=true \
component/test_
Github user agneya2001 commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-163939898
Sorry, I just started looking at PRs, next time I will elaborate on my LGTM.
For this particular one when a account is deleted it leaves the ssh key in
the us
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-163927873
@agneya2001 What made you LGTM this? the titel of the PR? The description
of the feature in the ticket? code review? testing?
---
If your project is set up fo
Github user agneya2001 commented on the pull request:
https://github.com/apache/cloudstack/pull/1212#issuecomment-163835153
LGTM.
---
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
GitHub user ustcweizhou opened a pull request:
https://github.com/apache/cloudstack/pull/1212
CLOUDSTACK-9136: remove ssh keypairs along with removing account
We also allow ROOT Admin to remove remained ssh keypairs of removed account
You can merge this pull request into a Git repos
23 matches
Mail list logo
