Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1057
---
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/1057#issuecomment-162287862
LGTM based on these tests:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a
tags=advanced,required_hardware=true \
component/test_
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-159176971
[1057.network.results.txt](https://github.com/apache/cloudstack/files/42440/1057.network.results.txt)
[1057.vpc.results.txt](https://github.com/apache/clou
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-159127988
@remibergsma, could you send me the stack trace of the test that failed?
---
If your project is set up for it, you can reply to this email and have your
r
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158664606
@remibergsma for me test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL
fails. That is not the one you meant is it? I also saw the failure you had.
---
If y
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158625241
:-)
---
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
e
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158624430
I was busy starting ;)
---
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 remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158624231
@DaanHoogland It could be a result of another failure. On master this test
passes, so maybe you can run just this test yourself and look at the details?
---
If
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158620306
thanks @remibergsma ```cidr is not formatted correctly: cloud``` sound like
a valid error message. not sure if it needs fixing in the test or in the code.
---
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158613419
@DaanHoogland @rafaelweingartner Much better now, just one failed test to
resolve:
Failed test: `TestVPCRouterOneNetwork`
```
Execute cmd:
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158499809
I understand why you closed the PR, but after a quick review, I thought
that I could help you, so letâs see if now everything is ok
---
If your project
GitHub user DaanHoogland reopened a pull request:
https://github.com/apache/cloudstack/pull/1057
Cwe 190
coverity warnings of this type adressed. Some where dismissed and maybe
with reason but it seemed possible to remove them and hence obligatory ;p
You can merge this pull request
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158486604
@rafaelweingartner thanks for picking this up. it is not a matter of giving
up but of priorities and I didn't want to keep it open knowing there was work
to do
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158434570
Hi @DaanHoogland,
Do not give up now, I really liked your PR.
Those errors can be easily explained. You changed the behavior of
com.cloud.utils.ne
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158400754
to many unexplained failures for such a small refactor.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user DaanHoogland closed the pull request at:
https://github.com/apache/cloudstack/pull/1057
---
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 feat
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158327637
@remibergsma running them myself as well, now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. I
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-158314116
FYI: @DaanHoogland My tests show 29 test failures. I will retest to be
sure, but please hold until this is done.
---
If your project is set up for it, you can
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44524995
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -775,11 +776,19 @@ public static String getSubNet(final String ip, final
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44520633
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -869,31 +878,44 @@ public static boolean isNetworkAWithinNetworkB(final
Strin
Github user wilderrodrigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44520446
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -869,31 +878,44 @@ public static boolean isNetworkAWithinNetworkB(final
St
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44519906
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -869,31 +878,44 @@ public static boolean isNetworkAWithinNetworkB(final
Strin
Github user wilderrodrigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44519815
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -869,31 +878,44 @@ public static boolean isNetworkAWithinNetworkB(final
St
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44515725
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -775,11 +776,19 @@ public static String getSubNet(final String ip, final
Stri
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44515734
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -869,31 +878,44 @@ public static boolean isNetworkAWithinNetworkB(final
Strin
Github user wilderrodrigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44515608
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -869,31 +878,44 @@ public static boolean isNetworkAWithinNetworkB(final
St
Github user wilderrodrigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1057#discussion_r44515580
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -775,11 +776,19 @@ public static String getSubNet(final String ip, final
S
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-155710622
@rafaelweingartner yes of course. you caught me pants down ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on G
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-155710538
I was just re-checking the code, what about creating a test case for
"netMaskFromCidr" method?
---
If your project is set up for it, you can reply to thi
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-155710144
That is it.
Everything looks good to me now
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on G
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-155708663
@rafaelweingartner sure, they're not needed but I'll add them for clarity.
---
If your project is set up for it, you can reply to this email and have your
repl
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-155595924
Got it @DaanHoogland.
What about using () to surround the â(long) 0xâ in lines 885,
925 and 931 as you did in 779.
---
If your project
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-155577386
@rafaelweingartner the cast was the reason for starting this. Coverity
complained about a 32 bit shift on an integer (31 bit + sign)
---
If your project is se
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1057#issuecomment-155517568
@DaanHoogland, pretty good job, you created the test cases and removed the
return null cases.
I just have one question, why you use a cast for the octa
GitHub user DaanHoogland opened a pull request:
https://github.com/apache/cloudstack/pull/1057
Cwe 190
coverity warnings of this type adressed. Some where dismissed and maybe
with reason but it seemed possible to remove them and hence obligatory ;p
You can merge this pull request i
35 matches
Mail list logo
