add a comment to the generated lock file

add a comment to the generated lock file 
Committed: https://code.google.com/p/dart/source/detail?r=17951

[Bob Nystrom, 2013-01-31 22:17:44]

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

https://codereview.chromium.org/12133002/diff/1/utils/pub/lock_file.dart#newc...
utils/pub/lock_file.dart:18: static final comment = "# Generated by pub. See:
http://pub.dartlang.org/doc/glossary.html#lockfile";
It's probably not worth making a constant for this. I'd just put the string
inline below.

https://codereview.chromium.org/12133002/diff/1/utils/pub/lock_file.dart#newc...
utils/pub/lock_file.dart:34: var parsed = loadYaml(contents.replaceAll(comment,
''));
This shouldn't be necessary. YAML supports comments and should ignore them
automatically.

https://codereview.chromium.org/12133002/diff/1/utils/pub/lock_file.dart#newc...
utils/pub/lock_file.dart:93: return '$comment $lockFileString';
I'd use a multiline string here:

return '''
# Generated by pub...
${json.stringify({'packages': packagesObj})}
''';

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
File utils/tests/pub/lock_file_test.dart (right):

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
utils/tests/pub/lock_file_test.dart:166:
expect(loadYaml(lockfile.serialize().replaceAll(LockFile.comment, '')), equals({
This shouldn't be needed. If it is, there's a bug in YAML! :)

[keertip, 2013-01-31 22:23:39]

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
File utils/tests/pub/lock_file_test.dart (right):

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
utils/tests/pub/lock_file_test.dart:166:
expect(loadYaml(lockfile.serialize().replaceAll(LockFile.comment, '')), equals({

If the comment is not removed, loadYaml fails and returns null

On 2013/01/31 22:17:44, Bob Nystrom wrote:
> This shouldn't be needed. If it is, there's a bug in YAML! :)

[keertip, 2013-01-31 22:31:40]

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
File utils/tests/pub/lock_file_test.dart (right):

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
utils/tests/pub/lock_file_test.dart:166:
expect(loadYaml(lockfile.serialize().replaceAll(LockFile.comment, '')), equals({
Guess that was because the comment should be on a separate line! Will make the
changes.
 
On 2013/01/31 22:23:39, keertip wrote:
> 
> If the comment is not removed, loadYaml fails and returns null
> 
> On 2013/01/31 22:17:44, Bob Nystrom wrote:
> > This shouldn't be needed. If it is, there's a bug in YAML! :)
>

[Bob Nystrom, 2013-01-31 22:33:48]

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
File utils/tests/pub/lock_file_test.dart (right):

https://codereview.chromium.org/12133002/diff/1/utils/tests/pub/lock_file_tes...
utils/tests/pub/lock_file_test.dart:166:
expect(loadYaml(lockfile.serialize().replaceAll(LockFile.comment, '')), equals({
On 2013/01/31 22:31:40, keertip wrote:
> Guess that was because the comment should be on a separate line!

That was my hunch too. :)

> Will make the changes.
>  
> On 2013/01/31 22:23:39, keertip wrote:
> > 
> > If the comment is not removed, loadYaml fails and returns null
> > 
> > On 2013/01/31 22:17:44, Bob Nystrom wrote:
> > > This shouldn't be needed. If it is, there's a bug in YAML! :)
> > 
>

[keertip, 2013-01-31 22:40:40]

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

https://codereview.chromium.org/12133002/diff/1/utils/pub/lock_file.dart#newc...
utils/pub/lock_file.dart:18: static final comment = "# Generated by pub. See:
http://pub.dartlang.org/doc/glossary.html#lockfile";
On 2013/01/31 22:17:44, Bob Nystrom wrote:
> It's probably not worth making a constant for this. I'd just put the string
> inline below.

Done.

https://codereview.chromium.org/12133002/diff/1/utils/pub/lock_file.dart#newc...
utils/pub/lock_file.dart:34: var parsed = loadYaml(contents.replaceAll(comment,
''));
Removed.

On 2013/01/31 22:17:44, Bob Nystrom wrote:
> This shouldn't be necessary. YAML supports comments and should ignore them
> automatically.

https://codereview.chromium.org/12133002/diff/1/utils/pub/lock_file.dart#newc...
utils/pub/lock_file.dart:93: return '$comment $lockFileString';
On 2013/01/31 22:17:44, Bob Nystrom wrote:
> I'd use a multiline string here:
> 
> return '''
> # Generated by pub...
> ${json.stringify({'packages': packagesObj})}
> ''';

Done.

[Bob Nystrom, 2013-01-31 22:42:31]

One nit, otherwise LGTM!

https://codereview.chromium.org/12133002/diff/9001/utils/pub/lock_file.dart
File utils/pub/lock_file.dart (right):

https://codereview.chromium.org/12133002/diff/9001/utils/pub/lock_file.dart#n...
utils/pub/lock_file.dart:16: 
Can you remove this empty line?

[keertip, 2013-01-31 22:53:05]

https://codereview.chromium.org/12133002/diff/9001/utils/pub/lock_file.dart
File utils/pub/lock_file.dart (right):

https://codereview.chromium.org/12133002/diff/9001/utils/pub/lock_file.dart#n...
utils/pub/lock_file.dart:16: 
On 2013/01/31 22:42:31, Bob Nystrom wrote:
> Can you remove this empty line?

Done.