ben-xD
commented
3 years ago I'm on the fence as to whether I'm truly in favour of this change, as the example app was originally designed to be very manually driven, which can be helpful when debugging as-yet-unidentified issues.
The example app can be painful and manual way to do QA testing, which has not worked effectively thus far, given the numerous bugs I've found in really basic areas, such as ChannelOptions: calling a method like channel.setOptions crashes the app. This happened because the method implementation was copy-pasted from an unrelated platform method, and not updated at all. 🙈 If it were tested, we can see the test coverage, and see that those lines were not executed (if they were we'd know they were flawed as they crash the app). If we rely on the example app, we rely on 2 things:
- Developers to implement all features (and SDK interfaces) in the example app
- Developers (or users) manually checking for regression
Problems: why I fixed it
- Small daily pain: It was only a small annoyance in my day to day development flow to press the button every time the app launches.
- Unrealistic code and logic: This realtime creation functionality would only indicate an error if the instance failed to be created. IMHO, this should not be shown in the UI as it did, as apps would not want to show realtime states to the user. They would use the errors to show an error screen. (we should do this too). The code I removed from the example app is unlikely to be in any production app. Basically, it is not a good example to set.
- Avoiding complexity and state: However, the main reason for this change was to avoid adding even more complexity in the app. The rest of the app doesn't have to check or show UIs to handle the case when the realtime or rest client hasn't been made. These clients need to be instantiated for any meaningful use of the example app anyway.
Big issues to focus on
- https://github.com/ably/ably-flutter/issues/212 The example app is poorly written, and may push users away. An example app != manually driven test bench. Setting a good example in the example app is very important to help users on board to the SDK. Implementing every interface that the library provides has to be balanced with a readable example app.
- The example app is for users, not for testing. The
main.dartin the example app is unreadable and confusing, and would benefit from a refactor and simplification. - If we want to demonstrate managing Realtime and rest clients in a more advanced fashion, we should use that in context: e.g. logging in and logging out, or in a guide.
- The example app is for users, not for testing. The
- https://github.com/ably/ably-flutter/issues/220 Simplifying the test application, perhaps even replacing the additional application in the repo with integration tests. Because of its complexity, we don't have any integration with the IDE, and have very custom concepts in the tests (e.g. "test modules"). Also, try reading the error messages from failing test in CI, it is quite difficult and tedious. The integration tests have been the biggest issue in the SDK for me.
IMHO Both these things are much more valuable (external + internal developer experience and Ably developer productivity) than this issue.
See my comment on @ben-xD's pull request where these buttons were removed.
┆Issue is synchronized with this Jira Task by Unito