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

Add rule 'no-promise-as-boolean' #4790

Merged
merged 14 commits into from Jul 29, 2019

Conversation

@guidsdo
Copy link
Contributor

commented Jul 10, 2019

This commit fixes #3983

PR checklist

  • Addresses an existing issue: fixes #3983
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

Added rule to prevent people from using a Promise in a boolean expression.

Is there anything you'd like reviewers to focus on?

No.

CHANGELOG.md entry:

"[new-rule] no-promise-as-boolean"

@palantirtech

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

Thanks for your interest in palantir/tslint, @guidsdo! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

guidojo
This commit fixes #3983
@guidsdo

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

Build is failing due to a bug in typescript@next, please ignore this.

Copy link
Collaborator

left a comment

Good start!

test/rules/no-promise-as-boolean/test.ts.lint Outdated Show resolved Hide resolved
src/rules/noPromiseAsBooleanRule.ts Outdated Show resolved Hide resolved
src/rules/noPromiseAsBooleanRule.ts Outdated Show resolved Hide resolved
src/rules/noPromiseAsBooleanRule.ts Show resolved Hide resolved
src/rules/noPromiseAsBooleanRule.ts Outdated Show resolved Hide resolved
src/rules/noPromiseAsBooleanRule.ts Outdated Show resolved Hide resolved
guidsdo and others added 11 commits Jul 17, 2019
Co-Authored-By: Josh Goldberg <joshuakgoldberg@outlook.com>
guidojo
guidojo
@guidsdo

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2019

@JoshuaKGoldberg fixed all the issues, please re-review

@guidsdo

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2019

@JoshuaKGoldberg can I expect you to look at this before the 1st of August?

@nomcopter

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

Would love to use this in tslint before it is too late!

@guidsdo

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2019

Would love to use this in tslint before it is too late!

The end is near

@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

commented Jul 26, 2019

Hi folks! The 1st of August deadline is for sending PRs. I'm a bit swamped now but promise to review this, even if it's after then! 💖

Copy link
Collaborator

left a comment

Wonderful, thanks so much for adding this @guidsdo!

src/rules/noPromiseAsBooleanRule.ts Outdated Show resolved Hide resolved
@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

commented Jul 27, 2019

Ah, I don't have permissions to make the one suggested change @guidsdo. Could you please commit that suggestion and merge the latest master into this PR? Once you do, we'll be good to merge! 🚀

guidsdo and others added 2 commits Jul 27, 2019
Co-Authored-By: Josh Goldberg <joshuakgoldberg@outlook.com>
@guidsdo

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2019

Ah, I don't have permissions to make the one suggested change @guidsdo. Could you please commit that suggestion and merge the latest master into this PR? Once you do, we'll be good to merge! 🚀

Thought I put that on, but thanks I applied the change and merged master into my branch :-)

@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

commented Jul 29, 2019

giphy

Thanks so much @guidsdo! Rule is looking great; let's get it in.

@JoshuaKGoldberg JoshuaKGoldberg merged commit a3a0b32 into palantir:master Jul 29, 2019
13 of 14 checks passed
13 of 14 checks passed
ci/circleci: testNext Your tests failed on CircleCI
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: checkout-code Your tests passed on CircleCI!
Details
ci/circleci: clean-lockfile Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: test2.1 Your tests passed on CircleCI!
Details
ci/circleci: test2.4 Your tests passed on CircleCI!
Details
ci/circleci: test2.7 Your tests passed on CircleCI!
Details
ci/circleci: test2.8 Your tests passed on CircleCI!
Details
ci/circleci: test2.9 Your tests passed on CircleCI!
Details
ci/circleci: test3.0 Your tests passed on CircleCI!
Details
ci/circleci: testRc Your tests passed on CircleCI!
Details
cla/palantir CLA signed on 2019-07-10 13:21 UTC+00:00
Details
@adidahiya adidahiya referenced this pull request Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.