On Tue, May 28, 2013 at 01:32:48PM -0400, Will Stevens wrote: > Hey All, > I am getting close to finishing up this integration, so I want to make sure > I understand the process and what is required for submitting my code for > review. > > I have read this and am comfortable with its content: > http://cloudstack.apache.org/develop/non-committer.html > > You can check out more details regarding this integration here: > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Palo+Alto+Firewall+Integration > > Please let me know if you feel I am missing anything on that page. That is > still a work in progress, but it does cover the functionality being added > pretty well. The screenshots are not complete yet, but they are at about > 90% right now. > > On that page I have linked a public repo which has a recent working version > of the code (not feature complete yet and still needs some clean up).
Thanks for doing that! > > Here are the questions that I have about the process: > - Do I need to include tests for this code? If so, is this documented > somewhere? Since this is an integration with an external device, how would > tests be written to pass without actually connecting to a device? 2 test types: 1 - unit tests using a mocking framework are needed for non-trivial logic (complex methods) 2 - integration tests using the marvin framework are the best method of providing automated testing of a specific integration. However, you might want to see if your functionality is *already* covered in the test suite. If you are only implementing a driver for a specific technology, it might be easy to just play a set of tests against an environment with that device enabled. > - There is a small limitation in core which did not have any dependancies > which I have fixed (Sheng and I have discussed this briefly). Basically, I > added support for multiple networks per account when the source nat type is > 'per account' with an external device. Question: Should I be submitting > two patches; one which only addresses this core fix (about 5 lines of code) > and one which addresses the addition of the palo_alto network plugin? Or, > should I submit it all as one patch? Best to do it as 2. Note in the new feature patch that it relies on the "core" patch. > - Since this is an integration with a 3rd party product; should I setup > a publicly accessible system where the functionality can be reviewed, or > should I work with Palo Alto to get demo licenses for their VM firewall > appliances and provide the reviewers licences to test the functionality? I > am not sure how this aspect should work, so let me know what the best > approach would be. We don't have a good model for this. Your demo license proposal sounds interesting though. Perhaps that's the model we *should* be using whenever possible? > > I think thats it. Please let me know if something is not clear or if you > feel I need to flush out some of the details somewhere. > > Thanks, > > Will
