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

Mean time recovery #897

Merged
merged 8 commits into from
Oct 7, 2020
Merged

Mean time recovery #897

merged 8 commits into from
Oct 7, 2020

Conversation

YoDaMa
Copy link
Contributor

@YoDaMa YoDaMa commented Sep 23, 2020

Checklist

Reference/Link to the issue solved with this PR (if any)

Description of the problem

Description of the solution

@YoDaMa YoDaMa marked this pull request as draft September 23, 2020 23:33
@YoDaMa YoDaMa marked this pull request as ready for review October 3, 2020 00:47
@anthonyvercolano
Copy link
Contributor

#!/usr/bin/env node

What is this?


Refers to: sdklab/longhaultests/src/iothub_longhaul.ts:1 in 84bae55. [](commit_id = 84bae55, deletion_comment = False)

// });
// }, 10000);
// }
// })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You wanted this commented out?

// debug('Message sent successfully')
// }
// })
// }, 2000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I'll remove.


function wait (ms) {
return new Promise ((resolve, reject) => {
setTimeout(resolve, ms);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we switch to 4 char indent?

"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "",
"license": "ISC",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wha?

}, 5000);
}); // wait for 1 seconds
debug('killing childServer')
childServer.kill();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seriously. This looks creepy. Can we do better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol sorry this is just the nomenclature instead of Master/Slave

@anthonyvercolano
Copy link
Contributor

Overall comment for the MTR. For the overall scheme about what is going on and how each part works is not documented very well.

Internally to each piece should be better documented about what it's individually trying to do to accomplish the overall task.

A much more extensive readme covering would be nice.

Copy link
Contributor

@anthonyvercolano anthonyvercolano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@YoDaMa
Copy link
Contributor Author

YoDaMa commented Oct 7, 2020

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@YoDaMa
Copy link
Contributor Author

YoDaMa commented Oct 7, 2020

/azp run node-canary

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@YoDaMa YoDaMa closed this Oct 7, 2020
@YoDaMa YoDaMa reopened this Oct 7, 2020
@YoDaMa
Copy link
Contributor Author

YoDaMa commented Oct 7, 2020

/azp run horton-node-gate

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@anthonyvercolano
Copy link
Contributor

/azp run node-canary

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@anthonyvercolano
Copy link
Contributor

/azp run node-canary

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@YoDaMa
Copy link
Contributor Author

YoDaMa commented Oct 7, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s), but failed to run 1 pipeline(s).

@YoDaMa YoDaMa merged commit 9585de3 into Azure:master Oct 7, 2020
@YoDaMa YoDaMa deleted the mean_time_recovery branch November 4, 2020 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants