Deployments context specs (and some improvements)
#1733
Conversation
joshk
commented
Jan 7, 2025
joshk
commented
- Add specs to all functions
- Fix a function return
- Simplify some function arguments
And some improvements and fixes seen while adding specs, like incorrect function returns, and simplifying some of the function arguments.
the firmware association is already preloaded
joshk
force-pushed
the
deployments-context-specs
branch
from
89fdfa2 to
6152d2d
Compare
|
|
||
| @spec get_deployment_device_count(integer()) :: term() | nil | ||
| def get_deployment_device_count(deployment_id) do | ||
| @spec get_device_count(Deployment.t()) :: term() | nil |
There was a problem hiding this comment.
I hate this return type but that's how Repo.aggregate/3 is typed? 😕 I would have assumed it would be :: non_neg_integer() | nil
There was a problem hiding this comment.
I totally agree with you, and sadly thats how it is typed. I could change it to non_neg_integer but I don't know if there is another possible term() that might be returned.
| @@ -14,7 +14,7 @@ defmodule NervesHubWeb.API.DeploymentController do | |||
| @whitelist_fields [:name, :org_id, :firmware_id, :conditions, :is_active] | |||
|
|
|||
| def index(%{assigns: %{product: product}} = conn, _params) do | |||
| deployments = Deployments.get_deployments_by_product(product.id) | |||
| deployments = Deployments.get_deployments_by_product(product) | |||
There was a problem hiding this comment.
If we had the choice, would we generally lean towards passing structs around or only the required bits?
There was a problem hiding this comment.
Well, gooooood question. I mostly changed this because the only use of this function was when we had the Product and I preferred the aesthetics of this more. I did initially create a version where the product or product id could be passed through, but since its only used in two liveviews, I removed the product id version.
