Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading function names #31

Closed
mabunixda opened this issue Sep 27, 2019 · 5 comments
Closed

Misleading function names #31

mabunixda opened this issue Sep 27, 2019 · 5 comments

Comments

@mabunixda
Copy link

Hi,

i am working on a terraform provider to configure the unifi controller.
I started with using your library but then realized that all functions are just reporting the stats and not the configuration endpoints.

I forked your lib and gonna do some rewrites based on the api stuff from node-unifi.

I would also like to create a PR after finishing the adding, but i struggle with if a renaming of the existing functions is better than creating wired function names on getSites() and rename the existing to getSitesStats() for example..

@davidnewhall
Copy link
Member

This library has been rather single purposed for a long time. I only use it to query statistics that get saved into InfluxDB. I recently removed the influx code from this library and put it back into the cli app. That makes this library less opinionated and provides a better base for adding new features.

Since Go is a heavily typed language and the UniFI API is a poorly typed data source, it gets difficult to reason about some pieces of information. Keep this in mind as you're developing.

Now on to your question. I haven't done any research into "controlling" the controller through the API, but it sounds like that's what you're looking to do. That said, I wonder what "GetSites()" should do in this new feature-filled library? You want to rename the existing functions. Can you explain what the new functions with the existing names will do? Is it possible to combine the stats with the meta data you're looking for? In other words, instead of GetSites() and GetSitesStats() Can we just have one method and fill the struct with both pieces of data?

I checked out https://github.com/mabunixda/unifi but I don't see any commits yet. I work best from examples, so if you can stab in some code for me to look at, it'll help me understand your goals.

Thanks!

@davidnewhall
Copy link
Member

Was hoping for a reply by now. 👀

@davidnewhall
Copy link
Member

Bump?

@mabunixda
Copy link
Author

I switched on using/testing another provider

@davidnewhall
Copy link
Member

kk np. That library looks pretty legit!

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

No branches or pull requests

2 participants