Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Restructure to remove dependency between logging and data collection #113

Open
shankari opened this issue Jun 8, 2016 · 1 comment
Open

Restructure to remove dependency between logging and data collection #113

shankari opened this issue Jun 8, 2016 · 1 comment

Comments

@shankari
Copy link
Contributor

shankari commented Jun 8, 2016

In response to user feedback, we don't stop showing alerts. But we still want to have an option to turn them on for debugging in case random stuff breaks down. My plan was to repurpose the existing simulate_user_interaction flag to do so. The problem is that then we have either:

  • logging depend upon data collection, which is ugh
  • or a bunch of copy pasted code in data collection + dependencies from all other modules into data collection, which is double ugh

I think that the real fix is to have a separate flag for showing debug messages in the UI. But that's going to take a lot more time to get right, so logging this and moving on.
shankari added a commit to shankari/cordova-unified-logger that referenced this issue Jun 8, 2016
@shankari
Turn off UI alerts unless the user wants them for debugging
f55fc2d 
User wants them == "simulateUserInteraction"
Note that introduces a new, ugly dependency.
Fixing that is tracked in
e-mission/e-mission-data-collection#113
@shankari shankari mentioned this issue Jun 8, 2016
Merged
@shankari
Copy link
Contributor Author

shankari commented Jun 8, 2016

On android, since the notification interface is not abused to do logging, this is much cleaner - mainly because none of the other plugins actually use . That might be a better long-term solution for ios as well.

shankari added a commit to shankari/e-mission-data-collection that referenced this issue Jun 8, 2016
@shankari
Turn off UI alerts unless the user wants them for debugging
2cc112c 
User wants them == "simulateUserInteraction"
Unlike iOS, this does not introduce a new dependency, it does have a lot of
repeated code, specially in the services when we try to transition to a new
state.

Fixing it is being tracked in
e-mission#113

Note that the iOS change changes the logging directly, so it is not checked
into this repo. The iOS change is at:
shankari/cordova-unified-logger@f55fc2d

This is an object case in the pitfalls of both approaches - we just need to
refactor and do them right on the second pass.
@shankari shankari mentioned this issue Jun 8, 2016
Merged
shankari added a commit to shankari/cordova-unified-logger that referenced this issue Jul 26, 2016
@shankari
Add support for displaying notifications
a0ba403 
Since we repurposed notification display for logging, and then turned it off
because it was too distracting, we didn't have a way to just display
notifications. This change adds a new method to display notifications, and
refactors existing code to invoke it when necessary. This seems to be a fix for
e-mission/e-mission-data-collection#113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@shankari