Skip to content

make sure helpers return strict string#438

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

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

Conversation

@dead-horse

@dead-horse dead-horse commented Apr 28, 2015

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

jonathanong commented Apr 28, 2015

Copy link
Copy Markdown
Member

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

@targos

targos commented Apr 28, 2015

Copy link
Copy Markdown
Contributor

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

@juliangruber

juliangruber commented Apr 28, 2015

Copy link
Copy Markdown
Contributor

LGTM

@juliangruber

juliangruber commented Apr 28, 2015

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

fengmk2 commented Apr 29, 2015

Copy link
Copy Markdown
Member

LGTM

@tj

tj commented Apr 29, 2015

Copy link
Copy Markdown
Member

👍

dead-horse added a commit that referenced this pull request Apr 29, 2015
@dead-horse
Merge pull request #438 from koajs/strict
e21fc8a 
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

dead-horse commented Apr 29, 2015

Copy link
Copy Markdown
Member Author

0.20.0 ?

@jonathanong

jonathanong commented Apr 30, 2015

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dead-horse @jonathanong @targos @juliangruber @fengmk2 @tj