Issue 11348268: Don't require users to copy-paste authorization codes when authorizing pub.

Issue 11348268: Don't require users to copy-paste authorization codes when authorizing pub. (Closed)

Created:
8 years ago by nweiz

Modified:
8 years ago

Reviewers:
Bob Nystrom

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Don't require users to copy-paste authorization codes when authorizing pub. BUG=6951 Committed: https://code.google.com/p/dart/source/detail?r=15468

Patch Set 1 #

Total comments: 10

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+158 lines, -63 lines)			Patch
M	utils/pub/oauth2.dart	View	2 chunks	+54 lines, -33 lines	6 comments	Download
M	utils/pub/utils.dart	View	2 chunks	+47 lines, -0 lines	4 comments	Download
M	utils/tests/pub/oauth2_test.dart	View	5 chunks	+28 lines, -17 lines	0 comments	Download
M	utils/tests/pub/test_pub.dart	View	3 chunks	+29 lines, -13 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Bob Nystrom

Couple nits. Otherwise LGTM. https://codereview.chromium.org/11348268/diff/1/utils/pub/oauth2.dart File utils/pub/oauth2.dart (right): https://codereview.chromium.org/11348268/diff/1/utils/pub/oauth2.dart#newcode83 utils/pub/oauth2.dart:83: return new Future.immediate(new Client( Can't ...

8 years ago (2012-11-28 19:35:41 UTC) #3

nweiz

8 years ago (2012-11-28 19:42:14 UTC) #4

https://codereview.chromium.org/11348268/diff/1/utils/pub/oauth2.dart
File utils/pub/oauth2.dart (right):

https://codereview.chromium.org/11348268/diff/1/utils/pub/oauth2.dart#newcode83
utils/pub/oauth2.dart:83: return new Future.immediate(new Client(
On 2012/11/28 19:35:41, Bob Nystrom wrote:
> Can't wait until Future unifies chain/transform...

Indeed.

https://codereview.chromium.org/11348268/diff/1/utils/pub/oauth2.dart#newcode152
utils/pub/oauth2.dart:152: server.addRequestHandler((request) => request.path ==
"/", (request, response) {
On 2012/11/28 19:35:41, Bob Nystrom wrote:
> Long line.

Done.

https://codereview.chromium.org/11348268/diff/1/utils/pub/oauth2.dart#newcode170
utils/pub/oauth2.dart:170: 'Go to $authUrl\n'
On 2012/11/28 19:35:41, Bob Nystrom wrote:
> "In a web browser, go to"?

Done.

https://codereview.chromium.org/11348268/diff/1/utils/pub/utils.dart
File utils/pub/utils.dart (right):

https://codereview.chromium.org/11348268/diff/1/utils/pub/utils.dart#newcode154
utils/pub/utils.dart:154: 
On 2012/11/28 19:35:41, Bob Nystrom wrote:
> Did you copy these from pkg/http? If so, leave a comment to that effect with a
> TODO to unify.

Done.

https://codereview.chromium.org/11348268/diff/1/utils/pub/utils.dart#newcode157
utils/pub/utils.dart:157: Uri addQueryParameters(Uri url, Map<String, String>
parameters) {
On 2012/11/28 19:35:41, Bob Nystrom wrote:
> "add" -> "set" since it replaces.

"set" seems like it implies that it mutates the URL, whereas this returns a new
one.

Expand Messages | Collapse Messages