Add FolderID to SetDashboard api request #67
Conversation
cmd/import-dashboards/main.go
Outdated
| @@ -66,7 +66,7 @@ func main() { | |||
| continue | |||
| } | |||
| c.DeleteDashboard(board.UpdateSlug()) | |||
| _, err := c.SetDashboard(board, false) | |||
| _, err := c.SetDashboard(board, 0, false) | |||
There was a problem hiding this comment.
Could we convert 0 into a constant with a name like const GeneralFolderId = 0 so that it would be clearer what's going on?
There was a problem hiding this comment.
Okay. I'll change that. Thanks
rest-dashboard.go
Outdated
| @@ -165,11 +165,12 @@ func (r *Client) SearchDashboards(query string, starred bool, tags ...string) ([ | |||
| // may be only loaded with HTTP API but not created or updated. | |||
| // | |||
| // Reflects POST /api/dashboards/db API call. | |||
| func (r *Client) SetDashboard(board Board, overwrite bool) (StatusMessage, error) { | |||
| func (r *Client) SetDashboard(board Board, folderId int, overwrite bool) (StatusMessage, error) { | |||
There was a problem hiding this comment.
Lets please avoid adding new parameters like that. Could you please add a type type SetDashboardParams func(values url.Values) just like in rest-annotation.go, convert this into a variadic function, and then iterate over them instead of adding this? We really want to avoid this.
Signed-off-by: Masudur Rahman <masud@appscode.com>
Signed-off-by: Masudur Rahman <masud@appscode.com>
masudur-rahman
force-pushed
the
add-folder-id
branch
from
15ca396
to
dfba69b
Compare
@GiedriusS, we can't use I think it will be a bit problem as we don't get Could you please check if the changes are okay ? |
There was a problem hiding this comment.
This is a tough one to call. We want to avoid becoming a frankstein with a bunch of different calling conventions but IMHO this makes sense. The SetDashboard could potentially gain a bunch of different options into the future and we want to add new stuff without breaking existing users. It's a good middle ground solution between complete extensibility via interface{} but zero type-safety and listing all of the possible options explicitly in a function call.
Two requests in-line. Thanks for working on this!
Signed-off-by: Masudur Rahman <masud@appscode.com>
|
Hey @GiedriusS, I have added the comment you suggested. Hope it's okay. |
okay |
Signed-off-by: Masudur Rahman masud@appscode.com