[webkit-reviews] review granted: [Bug 72243] Add a tool to analyze change logs : [Attachment 117741] Fixed per Eric's review

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 72243: Add a tool to analyze change logs
https://bugs.webkit.org/show_bug.cgi?id=72243

Attachment 117741: Fixed per Eric's review
https://bugs.webkit.org/attachment.cgi?id=117741&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=117741&action=review


I expect we could make this nicer with a few more rounds.  But this is OK to
go.  Thanks for hte patch.  Please fix my and leandro's comments before
landing.

> Tools/Scripts/webkitpy/tool/commands/analyzechangelog.py:96
> +	   print commands_dir

Do you wnat this printing?

> Tools/Scripts/webkitpy/tool/commands/analyzechangelog.py:137
> +	   if self._filename:
> +	       print

Hum?

> Tools/Scripts/webkitpy/tool/commands/analyzechangelog_unittest.py:65
> +	   test_json = {'array.json': [1, 2, 3, {'key': 'value'}],
'dictionary.json': {'somekey': 'somevalue', 'array': [4, 5]}}
> +	   sys_stdout = sys.stdout
> +	   try:
> +	       sys.stdout = filesystem.open_text_file_for_writing('stdout')
> +	       AnalyzeChangeLog._generate_jsons(filesystem, test_json, 'bar')
> +	       self.assertEqual(set(filesystem.files.keys()),
set(['bar/array.json', 'bar/dictionary.json', 'stdout']))
> +	   finally:
> +	       sys.stdout = sys_stdout

Huh?  do you want OutputCapture?  That's our testing way of overriding
sys.stdout.

> Tools/Scripts/webkitpy/tool/commands/analyzechangelog_unittest.py:148
> +	   sys_stdout = sys.stdout
> +	   try:
> +	       sys.stdout =
host.filesystem.open_text_file_for_writing('stdout')
> +	       analyzer = ChangeLogAnalyzer(host,
['mock-checkout/foo/ChangeLog'])
> +	       analyzer.analyze()
> +	   finally:
> +	       sys.stdout = sys_stdout

Again, outputcapture.