Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-221846391
Rebased against latest 4.7.
---
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 swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-221762730
CI is clean and everything is green. I need some code review on this one.
Thanks...
---
If your project is set up for it, you can reply to this email and have your
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-219037058
It would be useful for someone knowledgeable in the network internals to
elaborate on @jayapalu's comment. We currently do not use the feature he's
talking abou
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-218848727
tag:needsreview
---
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 t
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-218839655
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 0
Errors: 0
Duration: 8h 34m 12s
```
**Associ
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-218839846
Can I get some code review on this one? Thx...
---
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 ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-202811882
@cristofolini The idea was that the source NAT IP is always the first IP.
Not sure if the logic is correct. But apparently I need to correct the logic
for the c
Github user ProjectMoon commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57697669
--- Diff: server/src/com/cloud/network/router/CommandSetupHelper.java ---
@@ -778,10 +778,11 @@ public int compare(final PublicIpAddress o1, final
Pu
Github user cristofolini commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-202152926
@ProjectMoon According to that comment on line 781 in `CommandSetupHelper`
the conditional that follows is there to enable sourceNAT, yet you removed the
comma
Github user jayapalu commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57422486
--- Diff: server/src/com/cloud/network/router/CommandSetupHelper.java ---
@@ -778,10 +778,11 @@ public int compare(final PublicIpAddress o1, final
Publi
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57319355
--- Diff: server/test/com/cloud/network/IpAddressManagerTest.java ---
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF)
Github user ProjectMoon commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57317877
--- Diff: server/test/com/cloud/network/IpAddressManagerTest.java ---
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57313299
--- Diff: server/test/com/cloud/network/IpAddressManagerTest.java ---
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF)
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-200825346
It is always great to see parts of a bigger method being extracted to
smaller ones, and then test cases and java docs being used.
I believe the te
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57313534
--- Diff: server/src/com/cloud/network/IpAddressManagerImpl.java ---
@@ -1698,6 +1698,22 @@ public String acquireGuestIpAddress(Network network,
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-200345827
I added a basic test now and moved the method out @rafaelweingartner. Maybe
there are more scenarios that can be tested?
---
If your project is set up for it,
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57031859
--- Diff: server/src/com/cloud/network/IpAddressManagerImpl.java ---
@@ -1714,8 +1714,13 @@ public boolean applyStaticNats(List staticNats, bool
17 matches
Mail list logo
