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.

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

Fix devicestatus and treatments id delete #4088

Merged

Conversation

jpcunningh
Copy link
Collaborator

@jpcunningh jpcunningh commented Nov 21, 2018

Fix for #4080

@PieterGit
Copy link
Contributor

@jpcunningh Do you understand why you need to create a new variable and refer to that in the JSON
part of the code.
image

From reading the code I would assume that the new code is equivalent (but introduces a new temporary variable). Am I missing something?

@PieterGit PieterGit added this to the 0.11.0 milestone Nov 21, 2018
@PieterGit PieterGit added the bug label Nov 21, 2018
@PieterGit PieterGit requested a review from sulkaharo November 21, 2018 07:48
@ghost
Copy link

ghost commented Nov 21, 2018

Omg stop sending these threads. I hetbloke 50 emails a day. Stop stop stop please

@sulkaharo
Copy link
Member

@candreassen if you get emails from changes in Nigthscout, it means your account is actively "watching" the Nightscout repository and thus will get an email from every change. If you want the email to stop, go to the Github Nightscout page (for example, click the "view in Github" link in the email with this message) and find a button on top of the page that says "Unwatch" and either choose "Not Watching" or "Ignore" from the menu.

@sulkaharo
Copy link
Member

@PieterGit looks like calling the remove() method call with new ObjectID() results in the remove() call being called with a function pointer that returns the ObjectID, not the new ObjectID(), which causes the call to fail. Using the temporary variable fixes this as the remove() function is then called with ObjectID, as expected.

@jpcunningh
Copy link
Collaborator Author

Thank you for the explanation, @sulkaharo! I didn't know why one way worked vs. the other.

@PieterGit PieterGit merged commit cf7b40e into nightscout:dev Nov 21, 2018
@PieterGit
Copy link
Contributor

@sulkaharo thanks for the explanation. I hate merging stuff I don't understand.
@jpcunningh thanks for the fix

@jpcunningh jpcunningh deleted the fix-devicestatus-and-treatment-id-delete branch November 22, 2018 00:13
@PieterGit
Copy link
Contributor

The user documentation for this feature can be improved. I logged issue nightscout/documentation#2 for that.

@staceyc2578
Copy link

staceyc2578 commented Jan 7, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants