Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/935#issuecomment-148639781
  
    Hi @nvazquez 
    
    Thanks for your contribution. I'm not quite sure I understand the problem 
you are trying to solve, but that likely my own lack of NXS knowledge.
    The last time I changed this plugin (NiciraNvp), I've added an integration 
tests for checking is redirects within a cluster were being followed. You 
change probably doesn't impact that, but it would be great to make sure. Can 
you please run the test (`test/integration/smoke/test_nicira_controller.py`, 
let me know if you need help in getting the test running) in your setup and 
report on the results?
    In addition, it would be great of you document the problem you are trying 
to fix by adding a new integration tests (that fails before your change is 
applied and succeeds afterwards).
    
    A final remark about the code and the commits, please see my comments on 
certain lines, and consider rearranging your commits in such a way that what 
you did and/or the intention behind each commit become clear.
    
    Thanks again, and do let me know if I can hep you further.


---
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 enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to