Skip to content

Conversation

integrii
Copy link

I used this package for a recent project of mine and it was really helpful. I ended up having to expand this package with a bunch of different things, so I want to contribute back.

Some things worth noting:

  • Many fields are now omitempty so they dont overwrite parent profile settings if they are defaults
  • some new profile types are supported
  • a bunch of random bug fixes
  • a new Debug mode you can set by doing bigip.Debug = true

Let me know if this stuff is suitable. Unfortunately, this package does not have any real tests so I can't say that it will continue to pass your tests.

Thanks for creating this!

@jakauppila
Copy link
Contributor

I agree that breaking out the individual configuration objects into their own files would make management a little easier at this point, I think that would best be handled in a PR in itself.

That being said, there is a lot going on here. Breaking the changes down into manageable scoped PRs would make reviewing them a lot easier.

@integrii
Copy link
Author

Your request is totally reasonable. Unfortunately there are so many changes that I would have to look over my git history and make PRs for an hour. I may get to this someday, but probably not soon...

@zlesnr
Copy link
Collaborator

zlesnr commented Dec 19, 2017

What about nesting calls using the ltm api namespace in a folder called ltm to match. This would allow quick review about which files are associated with which api paths.

@jakauppila
Copy link
Contributor

@zlesnr I would agree with that; it should be modeled similarly how F5 structures their Python SDK:

https://github.com/F5Networks/f5-common-python/tree/development/f5/bigip

@scottdware
Copy link
Owner

@zlesnr Good idea. Definitely easier to keep track of things.

marsu-p pushed a commit to marsu-p/go-bigip that referenced this pull request Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants