-
-
Notifications
You must be signed in to change notification settings - Fork 207
Add sorting to device table and gw table #579
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
Conversation
Hi @ttulka, thanks, I agree this is a useful feature. One thing however which will cause ambiguity is the Therefore I think this should be turned into an enum. That would make it very explicit and would self-document what the available options are. As well it avoids run-time errors because of typos, because an incorrect enum value can be catched at compile time. That would probably also mean that an Would you mind updating your PR with these changes? |
Hi @brocaar thanks for your comments! |
The tests are failing because the code is not formatted using |
Improve enum usage and formatting
@brocaar Thanks a lot for your feedback :) |
We can use match to update the q variable. There is also no need to match for _, as this would hide unmatched OrderBy enum values.
Hi @ttulka and @schmifran , this morning re-reviewed your changed + I have made some updates to simplify some things (in my opinion) and align it with the existing code (e.g. implementing the One question, should we only add sorting for gateways and devices, or would you like to extend this PR to also include sorting for the other tables, now that we have the structure in place for this? |
Hi @brocaar Many thanks! Thus, I don't see much additional value in sorting other entities for now. Thanks again |
Thanks @ttulka and @schmifran, I have merged your work 👍 |
Co-authored-by: Franka Schmid <fra.schmid@rational-online.com> Co-authored-by: Orne Brocaar <info@brocaar.com>
No description provided.