Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Great! Thanks guys!
---
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
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@jburwell that is great; probably I have just read half of an email thread.
---
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 jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@serg38 I will be going through other PRs tomorrow that are due for merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If you
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@jburwell Thanks a lot.
---
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 wis
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@rafaelweingartner @serg38 is correct. Any committer may merge a PR so
long as the following conditions are met:
* At least 1 code review LGTM
* At least 1 test LGTM
*
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@rafaelweingartner . Thanks. I believe per current process any committer
can merge PRs as per below. We have so many PRs in merge ready state
Github user jburwell commented on the
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@serg38 I do not know how it is working right now this process; but, I
recall seeing an email saying that only RMs would be allowed to merge now. I
might be mistaken, I had lost track
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@rhtyd @jburwell @swill @koushik-das @rafaelweingartner @wido This PR has
enough of everything. Can one of the committers merge it?
---
If your project is set up for it, you can reply to this e
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@rhtyd @jburwell Just re-ran Marvin tests. All passes including
test_network_acl which fails without this fix.
[root@ussarlabcsmgt41 ~]# cat
/tmp//MarvinLogs/test_volumes_939G6N/results.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
LGTM thanks @nvazquez for the change. Can you help with testing,
unfortunately I don't have the resources (some infra issues) to run tests
across vmware and xenserver.
---
If your project is set
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
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
enabled and wishes so, or if the fe
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Thanks @rhtyd!
I rebased branch '4.9' and pushed some changes based on your last comment.
I had to change visibility of `com.cloud.utils.stringUtils.UTF8` from private
to public.
I als
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Minor improvements suggested, 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
enabl
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@rhtyd @jburwell Can this be 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 project does not have this feature
ena
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@jburwell Commit message amended. Is it ok like this? Yes, it should be ran
against VMware.
We also made some individual tests for `SshHelper.sshExecute` by adding a
test API method w
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@jburwell With this PR test_network_acl is successfull
[root@ussarlabcsmgt41 Aug_23_2016_16_49_32_8HYYKG]# tail results.txt
Test network ACL lists and items in VPC ... === TestName: test_n
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@jburwell I don't have a VMware setup, maybe when we got Trillian back I
could have a look.
---
If your project is set up for it, you can reply to this email and have your
reply appear o
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@nvazquez could you please amend your commit message to explain why/how
this fix addresses the defect?
Also, verification of this PR is to run ``test_network_acl`` against
VMware, corr
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@jburwell now both tests passed!
---
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
enabl
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@nvazquez The Travis build has failed. Can you investigate? Likely just
needs to be re-kicked.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Thanks @rhtyd @jburwell for reviewing!
@jburwell sure, now PR is targeted to 4.9 branch. Jenkins hadn't started
properly last time and failed, with last refactor it started properly. I also
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@nvazquez the Jenkins build failed. Could you please investigate and fix
any issues?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
I looked at the class, while it can have further refactoring improvements,
fwiw LGTM. /cc @jburwell
---
If your project is set up for it, you can reply to this email and have your
reply appear o
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Done, thanks @rhtyd for your help, it was really useful!
---
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 rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@nvazquez okay, there are many ways to do this. Here's how you can do it
fairly easily without merge conflicts:
git checkout
git format-patch -1 # this will create a .patch fi
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Hi @rhtyd, it would be nice to include it in 4.9. I tried rebasing branch
4.9 but it tries to update poms to 4.10 version, what do you suggest me to do?
---
If your project is set up for it, y
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
@nvazquez @serg38 does this also affect 4.9, if so please edit the base
branch to 4.9. We should have this in 4.9/lts release.
---
If your project is set up for it, you can reply to this email an
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
RMs please help review /cc @jburwell @karuturi
---
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 f
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1660
Thanks, this looks interesting. I'll try to get back to you on this.
---
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
29 matches
Mail list logo
