Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(208)

Issue 11565046: Don't log a user's credentials. (Closed)

Created:
8 years ago by nweiz
Modified:
8 years ago
Reviewers:
Bob Nystrom
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Don't log a user's credentials. Committed: https://code.google.com/p/dart/source/detail?r=16184

Patch Set 1 #

Patch Set 2 : Fix a bug. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -4 lines) Patch
M utils/pub/io.dart View 1 2 chunks +6 lines, -3 lines 2 comments Download
M utils/pub/oauth2.dart View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
nweiz
8 years ago (2012-12-14 21:54:54 UTC) #1
Bob Nystrom
I think you should flip the boolean. Otherwise LGTM. https://codereview.chromium.org/11565046/diff/3001/utils/pub/io.dart File utils/pub/io.dart (right): https://codereview.chromium.org/11565046/diff/3001/utils/pub/io.dart#newcode110 utils/pub/io.dart:110: ...
8 years ago (2012-12-14 22:22:01 UTC) #2
nweiz
8 years ago (2012-12-14 22:29:53 UTC) #3
https://codereview.chromium.org/11565046/diff/3001/utils/pub/io.dart
File utils/pub/io.dart (right):

https://codereview.chromium.org/11565046/diff/3001/utils/pub/io.dart#newcode110
utils/pub/io.dart:110: Future<File> writeTextFile(file, String contents,
{dontLogContents: false}) {
On 2012/12/14 22:22:01, Bob Nystrom wrote:
> How about: logContents: true

I'm not a big fan of the double negative here either, but "logContents: true"
really looks like it's saying that the contents should always be logged. A more
accurate positive name would be "allowContentsToBeLogged", but that seemed even
worse than "dontLogContents".

Powered by Google App Engine
This is Rietveld 408576698