Skip to content

make sure helpers return strict string#438

Merged
dead-horse merged 1 commit intomasterfrom
strict
Apr 29, 2015
Merged

make sure helpers return strict string#438
dead-horse merged 1 commit intomasterfrom
strict

Conversation

@dead-horse
Copy link
Copy Markdown
Member

not sure if this is look good to everyone, but it reduce lots of code like:

(this.charset || '').toLowerCase();
(this.get('ua') || '').indexOf('chrome');
(this.type || '').match(/^text/);

and this won't break your app if your are using !this.get('referer') etc.

@jonathanong
Copy link
Copy Markdown
Member

LGTM. can't see any possible breaking changes. anyone else?

@targos
Copy link
Copy Markdown
Contributor

targos commented Apr 28, 2015

LGTM but I think there are missing return ''; in request.get(field)

@juliangruber
Copy link
Copy Markdown
Contributor

LGTM

@juliangruber
Copy link
Copy Markdown
Contributor

Strictly it's still a breaking change because we didn't tell people to use lose equality checks, if someone is strictly comparing typeof this.get('referer') === 'undefined' it breaks. i'm not sure though if someone does that even.

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 29, 2015

LGTM

@tj
Copy link
Copy Markdown
Member

tj commented Apr 29, 2015

👍

dead-horse added a commit that referenced this pull request Apr 29, 2015
make sure helpers return strict string
@dead-horse dead-horse merged commit e21fc8a into master Apr 29, 2015
@dead-horse dead-horse deleted the strict branch April 29, 2015 07:25
@dead-horse
Copy link
Copy Markdown
Member Author

0.20.0 ?

@jonathanong
Copy link
Copy Markdown
Member

go for it!

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