-
Notifications
You must be signed in to change notification settings - Fork 302
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
[dashboards] Add support for dashboard list API v2 #374
Conversation
c1d23c3
to
d1e7727
Compare
d1e7727
to
ce18bc3
Compare
fbdad16
to
7d54b7b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we choose to have v2
as a sub variable within DashboardList
? Would it be better to have a DashboardListV2
such a way we can do api.DashboardListV2
directly? Also, did we consider having something like api.v2.DashboardList
?
Adding a DashboardListV2 class means that we would have to do the same for all other classes that would need to be versioned and it can become very messy at some point. We also wanted to keep the name DashboardList because we want users to use the latest version of our API at the end. Having DashboardListV2 could let users think that they can use either DashboardList or DashboardListV2, which we don't want. This is why we wanted to implement a solution that would allow, in the future, to manage API updates in a simple way. With this approach, you can specify your endpoint version easily.
In general, when it comes to versioning, adding the prefix 'v2' after 'api' means that the whole api has been updated to another version, which is not the case here since only 4 endpoints have been updated. We have therefore chosen to place 'v2' after DashboardList to avoid any confusion and also because it seems clear enough for the users. |
DashboardList.v2.get_items
,DashboardList.v2.add_items
,DashboardList.v2.update_items
andDashboardList.v2.delete_items
API calls