Code review feedback - Githubissues

glennmatthews commented 1 year ago

Test cases look like a good start!
- Seems like there's room for some common helper functions, such as checking that error != nil (or failing the test) and checking that the response is a 2xx (or failing the test).
- Bulk operations test looks to be missing bulk-create and bulk-partial-update (PATCH) operations coverage
- Single operations test looks to be missing partial-update (PATCH) coverage
- Probably don't need to test CRUD for all models but it's worth thinking about testing the various bespoke endpoints (devices/napalm, jobs/run, prefixes/available-prefixes, etc.) to confirm that they appear to work correctly as generated.
What's the plan for documenting the API of this SDK? Obviously there are patterns in the generated code, but does the user just have to do a search of nautobot.go to find the function they need, or is there some way that documentation can be provided?
Should the default nautobot_ver in tasks.py be updated every time there's a new Nautobot release?
What about the version in ci.yml?
Actually, are any/all of the tasks in tasks.py even relevant to this project? Would be good to remove ones that don't apply.
It would be good to regenerate development/nautobot_config.py with latest Nautobot version as I see a lot of configs being explicitly specified in this file that should normally be fine to leave as default - plus potentially problematic (if likely irrelevant to this particular project) legacy settings like CACHEOPS_ENABLED = True.

All in all, looks very promising!

chadell commented 1 year ago

I believe that one of the biggest issues is how hard would be to use it. So I would like to find some more user friendly way to create it... open to suggestions

chadell commented 1 year ago

nautobot / go-nautobot

Code review feedback #16