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

build: fix tslint issues related to promises #1651

Merged
merged 1 commit into from
Aug 28, 2018
Merged

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Aug 28, 2018

A follow-up for #1648.

After I disabled no-unused-variable check, tslint started to complain about places incorrectly using promises. Some of the warnings were legitimate and I fixed the code. Other cases were not valid, I fixed them by modifying tslint config and adding comments to disable the problematic rule for few source code lines.

It makes me wonder why tslint is not reporting these promise-related problems when no-unused-variable is enabled, but I can't be bothered to investigate.

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@bajtos bajtos self-assigned this Aug 28, 2018
@bajtos bajtos merged commit 0364b59 into master Aug 28, 2018
@bajtos bajtos deleted the fix/tslint-promises branch August 28, 2018 16:55
@@ -45,7 +45,7 @@ describe('RestServer (integration)', () => {

it('honors port binding after instantiation', async () => {
const server = await givenAServer({rest: {port: 80}});
await server.bind(RestBindings.PORT).to(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

@bajtos why was await removed here and other places as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because they were used incorrectly ... server.bind doesn't return a promise and as such the await wasn't needed.

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.

6 participants