Skip to content

Expose more loopback middleware for require - #834

Merged
raymondfeng merged 1 commit into
masterfrom
feature/expose-loopback-middleware
Nov 19, 2014
Merged

Expose more loopback middleware for require#834
raymondfeng merged 1 commit into
masterfrom
feature/expose-loopback-middleware

Conversation

@raymondfeng

Copy link
Copy Markdown
Member

/to @bajtos

We need a way to reference loopback.favicon and other middleware from middleware.json.

Comment threadserver/middleware/error-handler.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is IMHO rather silly. errorHandler is just an alias for require('errorhandler'). In your middleware.json, you can use errorhandler instead of loopback/error-handler.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I'm fine to remove error-handler.

@bajtosbajtos self-assigned this Nov 19, 2014
@bajtos

Copy link
Copy Markdown
Member

IMO this is the last pull request related to middleware that we need to land before we can release a new minor version.

To speed things up, I am proposing the following:

  • removeserver/middleware/error-handler.js
  • keep server/middleware/favicon.js as it is now
  • add serve-favicon to loopback dependencies
  • land this patch and release v2.8.0

@ritch do you have any opinion on this PR?

@raymondfeng

Copy link
Copy Markdown
MemberAuthor

@bajtos, I'll make the changes based on your proposal. It won't conflict with the discussion at strongloop/loopback-boot#70.

@raymondfeng
raymondfengforce-pushed the feature/expose-loopback-middleware branch from 5ea4ada to e56ec57CompareNovember 19, 2014 19:11
@raymondfeng

Copy link
Copy Markdown
MemberAuthor

Done.

Comment threadpackage.json Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any reason for pinning down the minor version via ~, instead of supporting all minor versions via ^?

@bajtos

Copy link
Copy Markdown
Member

One more comment, feel free to land the PR if you have convincing reasons why ~ should be used instead of ^.

@bajtosbajtos assigned raymondfeng and unassigned bajtosNov 19, 2014
@raymondfeng

Copy link
Copy Markdown
MemberAuthor

I'll merge the PR and fix the dependency conventions in a different one.

@raymondfeng
raymondfengforce-pushed the feature/expose-loopback-middleware branch from e56ec57 to a9b4df1CompareNovember 19, 2014 19:32
raymondfeng added a commit that referenced this pull request Nov 19, 2014
…eware
Expose more loopback middleware for require
@raymondfeng
raymondfeng merged commit 62637e8 into masterNov 19, 2014
@raymondfeng
raymondfeng deleted the feature/expose-loopback-middleware branch November 19, 2014 19:33
Sign up for freeto 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.

3 participants

@raymondfeng@bajtos@crandmck