Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@nvazquez now everything seems to be ok
LGTM.
Great job!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@rafaelweingartner thanks for reviewing again! Minor refactor pushed
---
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 blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Trillian test result (tid-973)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29259 seconds
Marvin logs:
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan test
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has
been kicked to run smoke tests
---
If your project is set up for it, you can reply to this email and have your
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Packaging result: âcentos6 âcentos7 âdebian. JID-614
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep
you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan package
---
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
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov I've rebased master branch, can we re-run tests on this PR?
---
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 user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Hi @rafaelweingartner, I've refactored the code instead of using
`rollBackState` as static. I think that using static variable could lead to a
problem if new methods are invoked from another
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@rhtyd @karuturi To fix errors in B.O in
test_02_list_snapshots_with_removed_data_store we need to merge PR1961 and then
adjust test_data on B.O side to have correct mapping for nfs2 label
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@nvazquez can you check if the failures in above test results are related
to your changes, thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Trillian test result (tid-896)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32396 seconds
Marvin logs:
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan test
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has
been kicked to run smoke tests
---
If your project is set up for it, you can reply to this email and have your
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Packaging result: âcentos6 âcentos7 âdebian. JID-537
---
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
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan package
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep
you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@serg38 I did, the code is great as always. However, I have a concern about
that `rollBackState` variable being static there. Because the
`DomainManagerImpl ` is a singleton, using
Github user serg38 commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@rafaelweingartner Can you review latest updates from @nvazquez . Since
tests are passing this PR will be ready for merging to 4.10
---
If your project is set up for it, you can reply to this
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@rafaelweingartner no problem, I should have mentioned about changing the
variable to static. I'll work on your last comments :)
---
If your project is set up for it, you can reply to this
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@nvazquez was the static declaration there before?! I am so sorry I missed
it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Trillian test result (tid-876)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35807 seconds
Marvin logs:
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@rafaelweingartner I think I got your point, I tried to keep code as
similar as it was before, by declaring `rollBackState` as static class variable
on line 114. This way inner `finally` block
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@nvazquez great work.
However, there is a catch there that I think you might have overlooked.
This problem is caused by the method extraction I suggested.
If you take a
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@rafaelweingartner thanks for reviewing! I extracted code to new methods
and also added unit tests for them
---
If your project is set up for it, you can reply to this email and have your
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has
been kicked to run smoke tests
---
If your project is set up for it, you can reply to this email and have your
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan test
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Packaging result: âcentos6 âcentos7 âdebian. JID-520
---
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
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan package
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep
you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Trillian test result (tid-810)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32603 seconds
Marvin logs:
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has
been kicked to run smoke tests
---
If your project is set up for it, you can reply to this email and have your
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan test
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
Packaging result: âcentos6 âcentos7 âdebian. JID-478
---
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
Github user blueorangutan commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep
you posted as I make progress.
---
If your project is set up for it, you can reply to this email and have your
Github user borisstoyanov commented on the issue:
https://github.com/apache/cloudstack/pull/1935
@blueorangutan package
---
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
37 matches
Mail list logo
