Add a shelf_web_socket package.

Add a shelf_web_socket package.

R=kevmoo@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36394

[kevmoo, 2014-05-20 21:54:49]

LGTM w/ comments

...especially the non-usage of crypto

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/README.md
File pkg/shelf_web_socket/README.md (right):

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/README....
pkg/shelf_web_socket/README.md:30: }
end block w/ ```

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
File pkg/shelf_web_socket/lib/src/web_socket_handler.dart (right):

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:9: import
'package:crypto/crypto.dart';
not used?

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:25: Response get _notFound
=> _htmlResponse(404, "404 Not Found",
Make this a (static? top-level?) method

Definitely not a property

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:85: assert(false);
Explain the flow here a bit in a comment. This is tough to reason about.

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:92: String
_chooseProtocol(Request request) {
Style nit: if methods don't access instance state, make them static (or better)
top-level.

...but until we have this resolved in a style guide...

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:116: Response
_htmlResponse(int statusCode, String title, String message) {
clarify that this method expects unescaped params

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/pubspec...
File pkg/shelf_web_socket/pubspec.yaml (right):

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/pubspec...
pkg/shelf_web_socket/pubspec.yaml:8: crypto: ">=0.9.0 <0.10.0"
Not user?

[nweiz, 2014-05-20 22:17:13]

Committed patchset #3 manually as r36394 (presubmit successful).

[nweiz, 2014-05-20 22:17:42]

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/README.md
File pkg/shelf_web_socket/README.md (right):

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/README....
pkg/shelf_web_socket/README.md:30: }
On 2014/05/20 21:54:49, kevmoo wrote:
> end block w/ ```

Done.

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
File pkg/shelf_web_socket/lib/src/web_socket_handler.dart (right):

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:9: import
'package:crypto/crypto.dart';
On 2014/05/20 21:54:49, kevmoo wrote:
> not used?

Done.

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:25: Response get _notFound
=> _htmlResponse(404, "404 Not Found",
On 2014/05/20 21:54:49, kevmoo wrote:
> Make this a (static? top-level?) method
> 
> Definitely not a property

Done.

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:85: assert(false);
On 2014/05/20 21:54:49, kevmoo wrote:
> Explain the flow here a bit in a comment. This is tough to reason about.

Done.

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:92: String
_chooseProtocol(Request request) {
On 2014/05/20 21:54:49, kevmoo wrote:
> Style nit: if methods don't access instance state, make them static (or
better)
> top-level.
> 
> ...but until we have this resolved in a style guide...

I disagree with this principle. Private methods should be defined in the same
place as the methods that call them, even if they don't (currently) use any
state.

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/lib/src...
pkg/shelf_web_socket/lib/src/web_socket_handler.dart:116: Response
_htmlResponse(int statusCode, String title, String message) {
On 2014/05/20 21:54:49, kevmoo wrote:
> clarify that this method expects unescaped params

Done.

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/pubspec...
File pkg/shelf_web_socket/pubspec.yaml (right):

https://codereview.chromium.org/297593003/diff/1/pkg/shelf_web_socket/pubspec...
pkg/shelf_web_socket/pubspec.yaml:8: crypto: ">=0.9.0 <0.10.0"
On 2014/05/20 21:54:49, kevmoo wrote:
> Not user?

Done.