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

Feat: sgc --retry (closes #65) #69

Merged
merged 9 commits into from
Dec 25, 2018
Merged

Feat: sgc --retry (closes #65) #69

merged 9 commits into from
Dec 25, 2018

Conversation

JPeer264
Copy link
Owner

@JPeer264 JPeer264 commented Nov 3, 2018

@aichbauer any suggestions on how to test this?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e895ec7 on feat/retry into b30c854 on master.

@coveralls
Copy link

coveralls commented Nov 3, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 04471dd on feat/retry into c6f7c0d on master.

@aichbauer
Copy link
Collaborator

Maybe you could make functions that read/writes the retry files and you call it in the cli.js so you have it separated from the cli input and then you could test if this function works properly?

Why are you rewriting some functions with async/await and others not?

@JPeer264
Copy link
Owner Author

JPeer264 commented Nov 3, 2018

Maybe you could make functions that read/writes the retry files and you call it in the cli.js so you have it separated from the cli input and then you could test if this function works properly?

Ya that would work.

Why are you rewriting some functions with async/await and others not?

Because top level await does not exist yet. And wrapping it into an own function would have been a little bit of an overhead. But ya maybe we should stick just either promises or async/await. Could rewrite it.

@JPeer264 JPeer264 mentioned this pull request Nov 29, 2018
@JPeer264
Copy link
Owner Author

@aichbauer tests added 👍

@JPeer264
Copy link
Owner Author

I also did a little refactoring. Unfortunately it is in this PR, as it was too difficult to put it in another PR.

@aichbauer
Copy link
Collaborator

I will take a look into it as soon as possible.

Copy link
Collaborator

@aichbauer aichbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@JPeer264 JPeer264 merged commit 786c6d3 into master Dec 25, 2018
@JPeer264 JPeer264 deleted the feat/retry branch February 16, 2019 05:44
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.

3 participants