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

Gave middlewares intuitive names #2

Merged
merged 2 commits into from Oct 23, 2015

Conversation

Projects
None yet
2 participants
@analog-nico
Contributor

analog-nico commented Oct 20, 2015

If you look into the express routing table you will find the names of the middleware functions. Before this change only "<anonymous>" was displayed.

Gave middlewares intuitive names
If you look into the express routing table you will find the names of the middleware functions. Before this change only '<anonymous>' was displayed.
@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Oct 20, 2015

Member

@analog-nico Woah, what's the Express routing table? How do I see that?

Member

EvanHahn commented Oct 20, 2015

@analog-nico Woah, what's the Express routing table? How do I see that?

@analog-nico

This comment has been minimized.

Show comment
Hide comment
@analog-nico

analog-nico Oct 21, 2015

Contributor

After you registered all your routes look at app._router.stack. There are a few libs that pretty print this information. In any case, I had a close look at my routing table yesterday in order to review my configuration. My code just doesn't give me a clear picture anymore. ;)

Contributor

analog-nico commented Oct 21, 2015

After you registered all your routes look at app._router.stack. There are a few libs that pretty print this information. In any case, I had a close look at my routing table yesterday in order to review my configuration. My code just doesn't give me a clear picture anymore. ;)

@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Oct 22, 2015

Member

I'm a little hesitant to do something just for a private feature of Express.

Is there any reason for the $ in the function names?

Member

EvanHahn commented Oct 22, 2015

I'm a little hesitant to do something just for a private feature of Express.

Is there any reason for the $ in the function names?

@analog-nico

This comment has been minimized.

Show comment
Hide comment
@analog-nico

analog-nico Oct 22, 2015

Contributor

No, there is no reason for $. Any name would be fine for me. BTW, all other helmet middlewares I use have a good name already. This one is the only one that returns anonymous functions. Giving them some names wouldn't hurt, am I right? Which names would you prefer?

Contributor

analog-nico commented Oct 22, 2015

No, there is no reason for $. Any name would be fine for me. BTW, all other helmet middlewares I use have a good name already. This one is the only one that returns anonymous functions. Giving them some names wouldn't hurt, am I right? Which names would you prefer?

@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Oct 22, 2015

Member

I'd be tempted to give them the same name: xXssProtection.

Member

EvanHahn commented Oct 22, 2015

I'd be tempted to give them the same name: xXssProtection.

@analog-nico

This comment has been minimized.

Show comment
Hide comment
@analog-nico

analog-nico Oct 22, 2015

Contributor

Done.

Contributor

analog-nico commented Oct 22, 2015

Done.

@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Oct 23, 2015

Member

Love it! Thank you.

Member

EvanHahn commented Oct 23, 2015

Love it! Thank you.

@EvanHahn EvanHahn closed this Oct 23, 2015

@EvanHahn EvanHahn reopened this Oct 23, 2015

EvanHahn added a commit that referenced this pull request Oct 23, 2015

Merge pull request #2 from analog-nico/patch-1
Gave middlewares intuitive names

@EvanHahn EvanHahn merged commit 6cdbf36 into helmetjs:master Oct 23, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@EvanHahn

This comment has been minimized.

Show comment
Hide comment
@EvanHahn

EvanHahn Oct 23, 2015

Member

Whoops—accidentally closed it. Reopened and merged.

Member

EvanHahn commented Oct 23, 2015

Whoops—accidentally closed it. Reopened and merged.

@analog-nico

This comment has been minimized.

Show comment
Hide comment
@analog-nico

analog-nico Oct 23, 2015

Contributor

Nice, thank you!

Contributor

analog-nico commented Oct 23, 2015

Nice, thank you!

@analog-nico analog-nico deleted the analog-nico:patch-1 branch Oct 23, 2015

@EvanHahn EvanHahn referenced this pull request Aug 5, 2016

Closed

Add list of contributors #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment