search

F5Networks / f5-openstack-image-prep

Contains scripts to prepare an F5® BIG-IP® VE image file to boot in OpenStack.
Apache License 2.0
0 stars 13 forks source link

patch-image tool needs doc cleanup and bugfixes #6

Closed pjbreaux closed 8 years ago

pjbreaux commented 8 years ago

@richbrowne

Issues: Addresses Issue #4

Problem: The patch image tool needs better functional structure and better documentation for usage. The startup script and it's functions need merging with those in perforce

Analysis: Added info to the main README.rst in this repo to demonstrate usage of the patch image tool. Merged in the new startup script changes.

pjbreaux commented 8 years ago

Okay @richbrowne, this is ready for review. Two things of interest here are that we renamed the openstack-functions directory so that makes the diff much larger. The other thing is that I had to comment out this line https://github.com/F5Networks/f5-openstack-image-prep/pull/6/files#diff-6ae8da551819e2a4ad1e77da84cd0c4eR97 in the startup script because it was bailing out here on bootup.

richbrowne commented 8 years ago

You did a good job here Paul. I think this is good and want to merge it, but there looks to be more opportunities to clean this up, by:

I wouldn't do these now, but: 1) Functionally combining the volume group managment. 2) Creating a main function (nice but not necessary), this could have a clear functional flow of what is going on. 3) Move all functions to the top of the file. There is one function def that seems to be defined in the middle of the file.

pjbreaux commented 8 years ago

Yes, I certainly saw those, and I have a ticket to address a full refactor of this script, but I didn't want to tackle that just yet, since I would just as soon turn it into a python tool.

pjbreaux commented 8 years ago

I'll take care of 1 and 3 now.

pjbreaux commented 8 years ago

Merging. This does not fix all issues brought up by @richbrowne, but it does allow a little more flexibility and readability in the patch image tool. The patch-image tool issue will remain open.