Cyclomatic complexity: globalize fetch dependency helper #621
Conversation
eshulman2
changed the title
eshulman2
changed the title
mandre
left a comment
mandre
left a comment
There was a problem hiding this comment.
Thanks, this is great! Would it also be possible to update existing controllers to use this new FetchDependency() function? This would help visualize the changes made to the template.
| Description: string(ptr.Deref(filter.Description, "")), | ||
| {{- range .ImportDependencies }} | ||
| {{ . }}: ptr.Deref({{ . | camelCase }}.Status.ID, ""), | ||
| {{ . }}ID: {{ . | camelCase }}ID, |
There was a problem hiding this comment.
Was this a bug in the original template? I'm unclean why this is changed while ImportDependencies remains identical.
There was a problem hiding this comment.
Most ID fields in gophercloud uses ResourceID in ListOpts from what I saw so the change seemed appropriate, I can change it back if needed
There was a problem hiding this comment.
Gophercloud is not really consistent, so either is fine. We should probably have avoided a functional change in a refactoring PR but that's not the end of the world because anyone using the scaffolding tool will have, at some point, to verify what fields the List() function takes, and LSP will highlight invalid fields anyway when your editor is configured correctly.
|
@mandre sure, I'll work on updating the rest of the controllers tomorrow |
|
/retest |
eshulman2
force-pushed
the
depHelper
branch
4 times, most recently
from
6709f79 to
7fcbfa9
Compare
- create a dependency helper replacing repetative parts of code - update template to use it
- Remove local getDependencyHelper and wrapper functions from server actuator - Use dependency.FetchDependency directly in CreateResource and ListOSResourcesForImport
winiciusallan
left a comment
winiciusallan
left a comment
There was a problem hiding this comment.
Overall, it looks very good to me. Good job, Ella!
mandre
left a comment
mandre
left a comment
There was a problem hiding this comment.
Really nice refactoring. Thanks a lot!
3 participants
To avoid cyclomatic complexity issue I added a global dependency helper that reduces code duplication and complexity across the code (similar to the one I added to the server controller). Now it should be easier to manage dependencies and the code duplication should be (somewhat reduced).