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 Linter errors blocking CI #798

Merged
merged 9 commits into from Apr 27, 2019

Conversation

Projects
None yet
4 participants
@alex-hall
Copy link
Contributor

commented Apr 5, 2019

This is mostly refactoring the test suite to use async/await instead of promises.

The only exception to this was the section where Q promises were being intentionally tested. I figured most people using those style promises aren't also using async/await in conjunction, so I added an eslint ignore there.

I can re-write those using async/await if there is interest.

Also, the catch block in the runOrRetry method didn't actually do anything, so I took the liberty of throwing that exception instead of returning an array nowhere.

alex-hall added some commits Apr 5, 2019

Fix linking errors around async await vs promises.
NOTE: Not all of the tests had assertions, so there may be some
Things incidentally passing here, but this is basically a 1:1
Replacement of promises to async/await, so there is no logic change here.
@alex-hall

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

Also, once this PR is accepted, we need to update dependencies.

I have done this locally and all tests still pass FYI..

Show resolved Hide resolved test/job.js
Show resolved Hide resolved test/job.js Outdated
Show resolved Hide resolved lib/utils/process-jobs.js Outdated

@alex-hall alex-hall closed this Apr 16, 2019

@alex-hall alex-hall reopened this Apr 16, 2019

@alex-hall

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

@wingsbob all comments should be fixed by now.

@wingsbob
Copy link
Contributor

left a comment

LGTM

@dandv dandv merged commit c17433c into agenda:master Apr 27, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (agenda) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.