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.

Sign up for GitHub

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

Jump to bottom

Add no else return tslint rule #1404

Merged
merged 2 commits into from
Apr 24, 2018
Merged

Add no else return tslint rule #1404

merged 2 commits into from
Apr 24, 2018

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Apr 23, 2018

This is a personal preference, any opposition?

@Tyriar Tyriar self-assigned this Apr 23, 2018
@Tyriar Tyriar requested a review from a team April 23, 2018 18:53
bgw
bgw approved these changes Apr 24, 2018
View reviewed changes
Copy link
Member

@bgw bgw left a comment

Choose a reason for hiding this comment

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

I don't really care either way, but more consistency is better I guess.

LucianBuzzo
LucianBuzzo approved these changes Apr 24, 2018
View reviewed changes
Copy link
Contributor

@LucianBuzzo LucianBuzzo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me, personally I think this makes the code more readable.

parisk
parisk approved these changes Apr 24, 2018
View reviewed changes
Copy link
Contributor

@parisk parisk left a comment

Choose a reason for hiding this comment

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

LGTM 🍰

@mofux
Copy link
Contributor

mofux commented Apr 24, 2018

LGTM 👍I generally prefer early returns over if else statements.

@Tyriar Tyriar added this to the 3.4.0 milestone Apr 24, 2018
@Tyriar Tyriar merged commit 8aa65d0 into xtermjs:master Apr 24, 2018
@Tyriar Tyriar deleted the no_else_return branch April 24, 2018 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LucianBuzzo LucianBuzzo LucianBuzzo approved these changes

@parisk parisk parisk approved these changes

@bgw bgw bgw approved these changes

Assignees

@Tyriar Tyriar

Labels
None yet
Projects
None yet
Milestone
3.4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Tyriar @mofux @bgw @parisk @LucianBuzzo