[plaso] Refactored options to CLI helpers #160

[plaso] Refactored options to CLI helpers #160

[aaronp, 2017-05-23 06:34:57]

On 2017/05/20 13:16:00, Joachim Metz wrote:

FYI: I started to take a look at this today.  I'll look again more closely
tomorrow.

[Joachim Metz, 2017-05-23 06:49:22]

great thx, just FYI this CL is a diffbase on top of
https://codereview.appspot.com/317630043/

[Joachim Metz, 2017-05-23 06:49:23]

great thx, just FYI this CL is a diffbase on top of
https://codereview.appspot.com/317630043/

[Joachim Metz, 2017-06-03 12:54:26]

Code updated.

[Joachim Metz, 2017-06-21 04:37:05]

Code updated.

[Joachim Metz, 2017-06-21 06:06:01]

Code updated.

[aaronp, 2017-06-23 01:33:19]

On 2017/06/21 06:06:01, Joachim Metz wrote:
> Code updated.

Ok -- Sorry again for the long delay on this review.  I started reviewing this,
but am not done yet.  In order to keep track of what's been reviewed vs. not, I
just added a "reviewed" comment to the files as appropriate.  Let me know if
that causes any issues with the review (I'm not sure if every comment needs to
be acknowledged or not).

I'll take another pass at this tomorrow.

[aaronp, 2017-06-23 01:34:48]

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extraction_tool.py
File plaso/cli/extraction_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extraction_tool...
plaso/cli/extraction_tool.py:1: # -*- coding: utf-8 -*-
reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/artifac...
File plaso/cli/helpers/artifact_definitions.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/artifac...
plaso/cli/helpers/artifact_definitions.py:1: # -*- coding: utf-8 -*-
Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/manager.py
File plaso/cli/helpers/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/manager...
plaso/cli/helpers/manager.py:1: # -*- coding: utf-8 -*-
Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/manager...
plaso/cli/helpers/manager.py:69: for helper_name, helper_class in
sorted(cls._helper_classes.items()):
Why add sorted() here?

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
File plaso/cli/helpers/yara_rules.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
plaso/cli/helpers/yara_rules.py:1: # -*- coding: utf-8 -*-
Reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
plaso/cli/helpers/yara_rules.py:60: u'Unable to read Yara rules file: {0:s} with
error: {1!s}'.format(
Cool, I didn't know you could do type conversion in a format string, :).

https://codereview.appspot.com/326740043/diff/60001/tests/output/manager.py
File tests/output/manager.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/output/manager.py#o...
tests/output/manager.py:106: manager.OutputManager.DeregisterOutput(TestOutput)
reviewed

[Joachim Metz, 2017-06-24 16:40:45]

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extraction_tool.py
File plaso/cli/extraction_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extraction_tool...
plaso/cli/extraction_tool.py:1: # -*- coding: utf-8 -*-
On 2017/06/23 01:34:47, aaronp wrote:
> reviewed

Acknowledged.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/artifac...
File plaso/cli/helpers/artifact_definitions.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/artifac...
plaso/cli/helpers/artifact_definitions.py:1: # -*- coding: utf-8 -*-
On 2017/06/23 01:34:48, aaronp wrote:
> Reviewed

Acknowledged.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/manager.py
File plaso/cli/helpers/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/manager...
plaso/cli/helpers/manager.py:1: # -*- coding: utf-8 -*-
On 2017/06/23 01:34:48, aaronp wrote:
> Reviewed

Acknowledged.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/manager...
plaso/cli/helpers/manager.py:69: for helper_name, helper_class in
sorted(cls._helper_classes.items()):
you're right, sorted is not needed here.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
File plaso/cli/helpers/yara_rules.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
plaso/cli/helpers/yara_rules.py:1: # -*- coding: utf-8 -*-
On 2017/06/23 01:34:48, aaronp wrote:
> Reviewed.

Acknowledged.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
plaso/cli/helpers/yara_rules.py:60: u'Unable to read Yara rules file: {0:s} with
error: {1!s}'.format(
On 2017/06/23 01:34:48, aaronp wrote:
> Cool, I didn't know you could do type conversion in a format string, :).

Acknowledged.

https://codereview.appspot.com/326740043/diff/60001/tests/output/manager.py
File tests/output/manager.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/output/manager.py#o...
tests/output/manager.py:106: manager.OutputManager.DeregisterOutput(TestOutput)
On 2017/06/23 01:34:48, aaronp wrote:
> reviewed

Acknowledged.

[Joachim Metz, 2017-06-24 16:42:40]

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
File plaso/cli/helpers/yara_rules.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/yara_ru...
plaso/cli/helpers/yara_rules.py:60: u'Unable to read Yara rules file: {0:s} with
error: {1!s}'.format(
technically it is not 100% type conversion

https://docs.python.org/2/library/string.html#formatspec
Two conversion flags are currently supported: '!s' which calls str() on the
value, and '!r' which calls repr().

[Joachim Metz, 2017-06-24 16:44:49]

Code updated.

[aaronp, 2017-06-29 01:32:22]

More partial review done.  No need for you to take another look yet.   I started
making some more note comments for myself in the review to help remember where
everything is moving around.  No need to acknowledge them; I can fix them up
before I'm done with the review.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/analysis_tool.py
File plaso/cli/analysis_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/analysis_tool.p...
plaso/cli/analysis_tool.py:1: # -*- coding: utf-8 -*-
Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/event_f...
File plaso/cli/helpers/event_filters.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/event_f...
plaso/cli/helpers/event_filters.py:1: # -*- coding: utf-8 -*-
Note to self:  Reviewed.
Came from psort_tool.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/languag...
File plaso/cli/helpers/language.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/languag...
plaso/cli/helpers/language.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
File plaso/cli/log2timeline_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:123: def _CheckStorageFile(self,
storage_file_path):
Note to self: Moved to cli/tool_options.py.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/pinfo_tool.py
File plaso/cli/pinfo_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/pinfo_tool.py#n...
plaso/cli/pinfo_tool.py:1: # -*- coding: utf-8 -*-
Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:103: def _CheckStorageFile(self, storage_file_path):
Note to self: Moved to tool_options.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:181: def _ParseFilterOptions(self, options):
Note to self: Moved to cli/helpers/event_filters.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:395: def AddFilterOptions(self, argument_group):
Note to self: Moved to cli/helpers/event_helper.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:434: def AddLanguageOptions(self, argument_group):
Note to self: Moved to helpers/language.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:611: # Add the output module options.
Note to self: Moved to cli/helpers/output_modules.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:126: def _CheckStorageFile(self, storage_file_path):
Note to self: Moved to cli/tool_options.py.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:157: def _GenerateStorageFileName(self):
Note to self: Came from cli/extract_analyze_tool.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py
File plaso/cli/tool_options.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:173: def _CheckStorageFile(self, storage_file_path):
Note to self: moved from each of cli/{log2timeline.py,psort_tool.py,psteal.py}

[Joachim Metz, 2017-06-29 15:11:06]

On 2017/06/29 01:32:22, aaronp wrote:
> More partial review done.  No need for you to take another look yet.   I
started
> making some more note comments for myself in the review to help remember where
> everything is moving around.  No need to acknowledge them; I can fix them up
> before I'm done with the review.

SG, there are some small changes I have to make to fix the tests as well

[Joachim Metz, 2017-06-30 04:46:13]

Code updated.

[Joachim Metz, 2017-07-04 18:22:22]

Code updated.

[Joachim Metz, 2017-07-08 15:58:03]

Code updated.

[aaronp, 2017-07-14 00:31:43]

Hopefully just one more pass until I'm done.  Again, ignore the "Note to self"
comments.  Only some small comments otherwise, and feel free to wait to look at
those until I'm done.

https://codereview.appspot.com/326740043/diff/60001/plaso/analysis/manager.py
File plaso/analysis/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/analysis/manager.py...
plaso/analysis/manager.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
File plaso/cli/extract_analyze_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
plaso/cli/extract_analyze_tool.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
plaso/cli/extract_analyze_tool.py:28: def _GenerateStorageFileName(self):
Note to self: Moved to psteal_tool

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
plaso/cli/extract_analyze_tool.py:69: def AddStorageFileOptions(self,
argument_group):
Note to self: Moved to storage_file helper.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/__init_...
File plaso/cli/helpers/__init__.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/__init_...
plaso/cli/helpers/__init__.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
File plaso/cli/helpers/analysis_plugins.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
plaso/cli/helpers/analysis_plugins.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
plaso/cli/helpers/analysis_plugins.py:69: if analysis_plugins and
analysis_plugins != u'list':
So where do these options actually get listed then?  I see the
list_output_modules gets set in the CLI tools, but I don't actually see where
it's used (other than to early exit the ParseOptions() method.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
plaso/cli/helpers/analysis_plugins.py:73: difference =
set(analysis_plugins).difference(plugin_names)
Small nit: The var names are a little ambiguous here... (i.e. analysis_plugins
vs. plugin_names).

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/data_lo...
File plaso/cli/helpers/data_location.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/data_lo...
plaso/cli/helpers/data_location.py:1: # -*- coding: utf-8 -*-
Note to self: moved from cli/tools.py
Reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/storage...
File plaso/cli/helpers/storage_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/storage...
plaso/cli/helpers/storage_file.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/hexdump.py
File plaso/cli/hexdump.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/hexdump.py#oldc...
plaso/cli/hexdump.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:557: analysis_group.add_argument(
Note to self: Moved to analysis_plugins.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:631: # Add the analysis plugin options.
Note to self: Moved to analysis_plugins.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py
File plaso/cli/tools.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#oldcode92
plaso/cli/tools.py:92: def _ClearScreen(self):
Note to self: No refs found.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#oldcod...
plaso/cli/tools.py:174: def _ParseDataLocationOption(self, options):
Note to self:  Moved to helpers/data_location.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py
File plaso/cli/tools.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#newcode1
plaso/cli/tools.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#newcod...
plaso/cli/tools.py:157: def _ParseLogFileOptions(self, options):
No helper manager for this?

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#newcod...
plaso/cli/tools.py:230: def _PromptUserForInput(self, input_text):
What's this for?

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool.py
File tests/cli/extraction_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool...
tests/cli/extraction_tool.py:130:
@shared_test_lib.skipUnlessHasTestFile([u'artifacts'])
Note to self: moved to tests/cli/helpers/analysis_plugins.py

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/analysi...
File tests/cli/helpers/analysis_plugins.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/analysi...
tests/cli/helpers/analysis_plugins.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/analysi...
tests/cli/helpers/analysis_plugins.py:68: # TODO: improve test coverage.
Maybe add testing '--analysis list' specifically to this TODO.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/artifac...
File tests/cli/helpers/artifact_definitions.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/artifac...
tests/cli/helpers/artifact_definitions.py:1: #!/usr/bin/python
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/artifac...
tests/cli/helpers/artifact_definitions.py:21: _EXPECTED_OUTPUT = u'\n'.join([
Maybe we should be using textwrap.dedent instead (for both this and other
similar tests)?

https://codereview.appspot.com/326740043/diff/60001/tests/cli/hexdump.py
File tests/cli/hexdump.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/hexdump.py#oldc...
tests/cli/hexdump.py:1: #!/usr/bin/python
Note to self: Reviewed.  Doesn't seem to be used anywhere.

[aaronp, 2017-07-14 23:59:53]

Getting closer, :).  Same notes/caveats apply as the last round.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/dynamic...
File plaso/cli/helpers/dynamic_output.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/dynamic...
plaso/cli/helpers/dynamic_output.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed.  Doesn't look like related code, but may have leaked
into this CL.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/filter_...
File plaso/cli/helpers/filter_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/filter_...
plaso/cli/helpers/filter_file.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed. Came from storage_media_tool.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/interfa...
File plaso/cli/helpers/interface.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/interfa...
plaso/cli/helpers/interface.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/output_...
File plaso/cli/helpers/output_modules.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/output_...
plaso/cli/helpers/output_modules.py:1: # -*- coding: utf-8 -*-
Note to self: Came from psort_tool.py.  Reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/output_...
plaso/cli/helpers/output_modules.py:44: if u'-o' in arguments:
This 'list' arg parsing pattern exists in a lot of places.  We might want to
refactor it out at some point.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/status_...
File plaso/cli/helpers/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/status_...
plaso/cli/helpers/status_view.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed. Moved from psort_tool.py,log2timeline.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/tempora...
File plaso/cli/helpers/temporary_directory.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/tempora...
plaso/cli/helpers/temporary_directory.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed. Moved from psort_tool.py,log2timeline_tool.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/image_export_to...
File plaso/cli/image_export_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/image_export_to...
plaso/cli/image_export_tool.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
File plaso/cli/log2timeline_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:286: self._temporary_directory = getattr(options,
u'temporary_directory', None)
Note to self: Moved to temporary_directory.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:338: argument_group.add_argument(
Note to self: Moved to temporary_directory.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:340: self._temporary_directory = getattr(options,
u'temporary_directory', None)
Note to self: Moved to temporary_directory.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:475: argument_group.add_argument(
Note to self: Moved to temporary_directory.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:571: info_group.add_argument(
Note to self: Moved to helpers/status_view.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:596: output_group.add_argument(
Note to self: Moved to output_modules.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:729: self._output_format = getattr(options,
u'output_format', None)
Note to self: Moved to output_modules.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view.py
File plaso/cli/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view.py#...
plaso/cli/status_view.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view_too...
File plaso/cli/status_view_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view_too...
plaso/cli/status_view_tool.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.  Moved to status_view.py previous CL.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
File plaso/cli/storage_media_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
plaso/cli/storage_media_tool.py:317: def _ParseFilterOptions(self, options):
Note to self: Moved to filter_file.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
plaso/cli/storage_media_tool.py:982: def AddFilterOptions(self, argument_group):
Note to self: Moved to filter_file.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py
File plaso/cli/tools.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#oldcode92
plaso/cli/tools.py:92: def _ClearScreen(self):
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: No refs found.

Actually, this is moved to status_view.py ...

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#oldcod...
plaso/cli/tools.py:313: def AddDataLocationOption(self, argument_group):
Note to self: Moved to data_location.py

https://codereview.appspot.com/326740043/diff/60001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/output/manager.py
File plaso/output/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/output/manager.py#n...
plaso/output/manager.py:1: # -*- coding: utf-8 -*-
Note to self: Reviewed

https://codereview.appspot.com/326740043/diff/60001/plaso/parsers/manager.py
File plaso/parsers/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/parsers/manager.py#...
plaso/parsers/manager.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/analysis/manager.py
File tests/analysis/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/analysis/manager.py...
tests/analysis/manager.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/analysis_tool.py
File tests/cli/analysis_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/analysis_tool.p...
tests/cli/analysis_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extract_analyze...
File tests/cli/extract_analyze_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extract_analyze...
tests/cli/extract_analyze_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
File tests/cli/helpers/output_modules.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
tests/cli/helpers/output_modules.py:1: #!/usr/bin/python
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
tests/cli/helpers/output_modules.py:74: options = cli_test_lib.TestOptions()
nit: I would separate these into at least two tests.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
tests/cli/helpers/output_modules.py:82: with
self.assertRaises(errors.BadConfigOption):
I guess this tests that an output file is required?  If this was a separate test
with a descriptive name it would be easier to read what's going on.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/status_...
File tests/cli/helpers/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/status_...
tests/cli/helpers/status_view.py:1: #!/usr/bin/python
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/tempora...
File tests/cli/helpers/temporary_directory.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/tempora...
tests/cli/helpers/temporary_directory.py:1: #!/usr/bin/python
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py
File tests/cli/psort_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#n...
tests/cli/psort_tool.py:118: # TODO: add test for _PrintStatusHeader.
This went away when it migrated to status_view.py.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view.py
File tests/cli/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view.py#...
tests/cli/status_view.py:1: #!/usr/bin/python
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tools.py
File tests/cli/tools.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tools.py#newcode1
tests/cli/tools.py:1: #!/usr/bin/python
Note to self: Reviewed.

[Joachim Metz, 2017-07-15 10:26:11]

Code updated.

[Joachim Metz, 2017-07-17 18:57:11]

> Getting closer, :).  Same notes/caveats apply as the last round.

SG, I'll do a bulk update when you're done. I'm making some merge changes in the
mean time.

[aaronp, 2017-07-26 05:00:54]

Getting so close.  I plan to finish this tomorrow.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
File plaso/cli/log2timeline_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:194: def _GetOutputModulesInformation(self):
Note to self: Moved to cli/tool_options.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:357: def ListOutputModules(self):
Note to self:  Moved to tool_options.py.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:189: def _ParseOutputModuleOptions(self, options):
Note to self: Adapted and moved to cli/tool_options.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:67: DESCRIPTION = textwrap.dedent(u'\n'.join([
(Sorry, I know this is not from your CL, but wanted to ask anyway)

Huh?  Is this code generated or something?  Why do textwrap.dedent(), and then
bother to join an inline array?  Is this to get around 80char linting errors or
something?

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py
File plaso/cli/tool_options.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:76: def _CreateOutputModule(self, options):
Note to self: moved from psteal_tool.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:116: missing_parameters =
output_module.GetMissingArguments()
The changes in this method are good ones, but a good example of things that
could have been done in a second PR to make this PR easier to review since a lot
of the other code in this method was just moved from another method.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:135: def _GetOutputModulesInformation(self):
Note to self: Moved from log2timeline_tool.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:147: def ListOutputModules(self):
Note to self: moved from log2timeline_tool.py

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/data_lo...
File tests/cli/helpers/data_location.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/data_lo...
tests/cli/helpers/data_location.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/dynamic...
File tests/cli/helpers/dynamic_output.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/dynamic...
tests/cli/helpers/dynamic_output.py:1: #!/usr/bin/python
Note to self: Reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/event_f...
File tests/cli/helpers/event_filters.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/event_f...
tests/cli/helpers/event_filters.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/filter_...
File tests/cli/helpers/filter_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/filter_...
tests/cli/helpers/filter_file.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/languag...
File tests/cli/helpers/language.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/languag...
tests/cli/helpers/language.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/manager.py
File tests/cli/helpers/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/manager...
tests/cli/helpers/manager.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/storage...
File tests/cli/helpers/storage_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/storage...
tests/cli/helpers/storage_file.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/yara_ru...
File tests/cli/helpers/yara_rules.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/yara_ru...
tests/cli/helpers/yara_rules.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/log2timeline_to...
File tests/cli/log2timeline_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/log2timeline_to...
tests/cli/log2timeline_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view_too...
File tests/cli/status_view_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view_too...
tests/cli/status_view_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

[aaronp, 2017-07-26 22:06:10]

Ok.  Really sorry for the long review.  Finally finished.  I have some comments
below on things that would make a PR like this easier to review (and thus
faster, :).

LGTM, just a few small notes and some questions.  If you have an easy way to
bulk-update the comments, feel free to do so.  Otherwise I can go through and
clean up my mess of "to self" comments.

Some hints on making PRs like this easier to review.
- Save unrelated refactoring (ie. var renaming, etc) for other PRs.  It makes it
easer to evaluate when a whole method has moved when there aren't other changes
there.
- Break into smaller chunks to make it easier to review (e.g. in this case we
could have broken up the helpers to be moved in smaller groups).
- If some code was copied in a previous PR as part of a migration, and this PR
completes the migration by deleting the original source, note it in the
description.
- If something is being deleted completely and not migrated, note it in the
description (and maybe save it for a different PR).

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
File plaso/cli/log2timeline_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:514: serializer_format = getattr(
Note to self: Moved from extraction_tool.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:239: def _ParseLanguageOptions(self, options):
Note to self: Moved to helpers/language.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:253: def _ParseOutputModuleOptions(self, options):
Note to self: Moved to tool_options.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:367: def _PromptUserForInput(self, input_text):
Note to self: Moved to tools.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:379: def AddAnalysisPluginOptions(self, argument_group,
plugin_names):
Note to self: Moved to tool_options

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:489: def ListAnalysisPlugins(self):
Note to self: Moved to tool_options.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:517: def ListOutputModules(self):
Note to self: Moved to tool_options.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#n...
plaso/cli/psort_tool.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:425: self.AddTimeZoneOption(output_group)
I guess this is a new option?  It doesn't seem like it was set on psteal before?
 Or did I just miss it somewhere?

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
File plaso/cli/storage_media_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
plaso/cli/storage_media_tool.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py
File plaso/cli/tool_options.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:1: # -*- coding: utf-8 -*-
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:46: def ListAnalysisPlugins(self):
Note to self: Moved from psort.py

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool.py
File tests/cli/extraction_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool...
tests/cli/extraction_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py
File tests/cli/psort_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#o...
tests/cli/psort_tool.py:346: def testListAnalysisPlugins(self):
Note to self: Moved to test*/tool_options.py

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#o...
tests/cli/psort_tool.py:401: def testListOutputModules(self):
Note to self: Moved to tests*/tool_options.py

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py
File tests/cli/psort_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#n...
tests/cli/psort_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psteal_tool.py
File tests/cli/psteal_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psteal_tool.py#...
tests/cli/psteal_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psteal_tool.py#...
tests/cli/psteal_tool.py:36: test_tool._source_path = u'/test/storage/path'
Lots of duplicate code here.   Seems like a candidate for a helper test function
that just takes a path and the filename parameter.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/storage_media_t...
File tests/cli/storage_media_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/storage_media_t...
tests/cli/storage_media_tool.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py
File tests/cli/tool_options.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:1: #!/usr/bin/python
Note to self: reviewed.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:16: """Tool to test the analysis plugin options."""
Adding a 'pass' here (and other empty classes) seems a little more readable to
me, but I'm not sure if there is a style guide entry for that.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:22: def testListAnalysisPlugins(self):
Note to self: Moved from tests*/psort.py

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:61: def testGetOutputModulesInformation(self):
Note to self: moved from tests*/log2timeline.py

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:72: def testListOutputModules(self):
Note to self: Moved from tests*/psort_tool.py

[Joachim Metz, 2017-07-27 04:02:09]

thx, I opt ot discuss process improvements outside of this CL.

[Joachim Metz, 2017-07-27 04:02:09]

thx, I opt ot discuss process improvements outside of this CL.

[Joachim Metz, 2017-07-27 05:43:56]

https://codereview.appspot.com/326740043/diff/60001/plaso/analysis/manager.py
File plaso/analysis/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/analysis/manager.py...
plaso/analysis/manager.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: Reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/analysis_tool.py
File plaso/cli/analysis_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/analysis_tool.p...
plaso/cli/analysis_tool.py:1: # -*- coding: utf-8 -*-
On 2017/06/29 01:32:22, aaronp wrote:
> Reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
File plaso/cli/extract_analyze_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
plaso/cli/extract_analyze_tool.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
plaso/cli/extract_analyze_tool.py:28: def _GenerateStorageFileName(self):
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: Moved to psteal_tool

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/extract_analyze...
plaso/cli/extract_analyze_tool.py:69: def AddStorageFileOptions(self,
argument_group):
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: Moved to storage_file helper.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/__init_...
File plaso/cli/helpers/__init__.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/__init_...
plaso/cli/helpers/__init__.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
File plaso/cli/helpers/analysis_plugins.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
plaso/cli/helpers/analysis_plugins.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
plaso/cli/helpers/analysis_plugins.py:69: if analysis_plugins and
analysis_plugins != u'list':
This is done in the tools. They check if there are list options that need to be
shown.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
plaso/cli/helpers/analysis_plugins.py:73: difference =
set(analysis_plugins).difference(plugin_names)
done

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/analysi...
plaso/cli/helpers/analysis_plugins.py:73: difference =
set(analysis_plugins).difference(plugin_names)
made some small changes

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/data_lo...
File plaso/cli/helpers/data_location.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/data_lo...
plaso/cli/helpers/data_location.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: moved from cli/tools.py
> Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/dynamic...
File plaso/cli/helpers/dynamic_output.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/dynamic...
plaso/cli/helpers/dynamic_output.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:51, aaronp wrote:
> Note to self: Reviewed.  Doesn't look like related code, but may have leaked
> into this CL.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/event_f...
File plaso/cli/helpers/event_filters.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/event_f...
plaso/cli/helpers/event_filters.py:1: # -*- coding: utf-8 -*-
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self:  Reviewed.
> Came from psort_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/filter_...
File plaso/cli/helpers/filter_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/filter_...
plaso/cli/helpers/filter_file.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Reviewed. Came from storage_media_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/interfa...
File plaso/cli/helpers/interface.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/interfa...
plaso/cli/helpers/interface.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/languag...
File plaso/cli/helpers/language.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/languag...
plaso/cli/helpers/language.py:1: # -*- coding: utf-8 -*-
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/output_...
File plaso/cli/helpers/output_modules.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/output_...
plaso/cli/helpers/output_modules.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Came from psort_tool.py.  Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/output_...
plaso/cli/helpers/output_modules.py:44: if u'-o' in arguments:
yes, the idea is to move these into the helper first. I'll add a TODO

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/status_...
File plaso/cli/helpers/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/status_...
plaso/cli/helpers/status_view.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Reviewed. Moved from psort_tool.py,log2timeline.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/storage...
File plaso/cli/helpers/storage_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/storage...
plaso/cli/helpers/storage_file.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: Reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/tempora...
File plaso/cli/helpers/temporary_directory.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/helpers/tempora...
plaso/cli/helpers/temporary_directory.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Reviewed. Moved from psort_tool.py,log2timeline_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/hexdump.py
File plaso/cli/hexdump.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/hexdump.py#oldc...
plaso/cli/hexdump.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/image_export_to...
File plaso/cli/image_export_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/image_export_to...
plaso/cli/image_export_tool.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
File plaso/cli/log2timeline_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:123: def _CheckStorageFile(self,
storage_file_path):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Moved to cli/tool_options.py.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:194: def _GetOutputModulesInformation(self):
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: Moved to cli/tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:286: self._temporary_directory = getattr(options,
u'temporary_directory', None)
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to temporary_directory.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:338: argument_group.add_argument(
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to temporary_directory.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:357: def ListOutputModules(self):
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self:  Moved to tool_options.py.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
File plaso/cli/log2timeline_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:1: # -*- coding: utf-8 -*-
On 2017/07/26 22:06:08, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/log2timeline_to...
plaso/cli/log2timeline_tool.py:514: serializer_format = getattr(
On 2017/07/26 22:06:08, aaronp wrote:
> Note to self: Moved from extraction_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/pinfo_tool.py
File plaso/cli/pinfo_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/pinfo_tool.py#n...
plaso/cli/pinfo_tool.py:1: # -*- coding: utf-8 -*-
On 2017/06/29 01:32:22, aaronp wrote:
> Reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:103: def _CheckStorageFile(self, storage_file_path):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Moved to tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:181: def _ParseFilterOptions(self, options):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Moved to cli/helpers/event_filters.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:239: def _ParseLanguageOptions(self, options):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved to helpers/language.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:253: def _ParseOutputModuleOptions(self, options):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved to tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:340: self._temporary_directory = getattr(options,
u'temporary_directory', None)
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to temporary_directory.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:367: def _PromptUserForInput(self, input_text):
On 2017/07/26 22:06:08, aaronp wrote:
> Note to self: Moved to tools.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:379: def AddAnalysisPluginOptions(self, argument_group,
plugin_names):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved to tool_options

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:395: def AddFilterOptions(self, argument_group):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Moved to cli/helpers/event_helper.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:434: def AddLanguageOptions(self, argument_group):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Moved to helpers/language.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:475: argument_group.add_argument(
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to temporary_directory.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:489: def ListAnalysisPlugins(self):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved to tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:517: def ListOutputModules(self):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved to tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:557: analysis_group.add_argument(
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: Moved to analysis_plugins.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:571: info_group.add_argument(
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to helpers/status_view.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:596: output_group.add_argument(
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to output_modules.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:611: # Add the output module options.
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Moved to cli/helpers/output_modules.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:631: # Add the analysis plugin options.
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: Moved to analysis_plugins.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#o...
plaso/cli/psort_tool.py:729: self._output_format = getattr(options,
u'output_format', None)
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to output_modules.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py
File plaso/cli/psort_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psort_tool.py#n...
plaso/cli/psort_tool.py:1: # -*- coding: utf-8 -*-
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:126: def _CheckStorageFile(self, storage_file_path):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Moved to cli/tool_options.py.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:189: def _ParseOutputModuleOptions(self, options):
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: Adapted and moved to cli/tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py
File plaso/cli/psteal_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:1: # -*- coding: utf-8 -*-
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:67: DESCRIPTION = textwrap.dedent(u'\n'.join([
let me check with romain, I've added a TODO for now

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:157: def _GenerateStorageFileName(self):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: Came from cli/extract_analyze_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/psteal_tool.py#...
plaso/cli/psteal_tool.py:425: self.AddTimeZoneOption(output_group)
Correct, I've been adding options available in log2timeline.py to psteal.py to
make it more feature equivalent / complete to log2timeline.py

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view.py
File plaso/cli/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view.py#...
plaso/cli/status_view.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view_too...
File plaso/cli/status_view_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/status_view_too...
plaso/cli/status_view_tool.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:51, aaronp wrote:
> Note to self: reviewed.  Moved to status_view.py previous CL.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
File plaso/cli/storage_media_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
plaso/cli/storage_media_tool.py:317: def _ParseFilterOptions(self, options):
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to filter_file.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
plaso/cli/storage_media_tool.py:982: def AddFilterOptions(self, argument_group):
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to filter_file.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
File plaso/cli/storage_media_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/storage_media_t...
plaso/cli/storage_media_tool.py:1: # -*- coding: utf-8 -*-
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py
File plaso/cli/tool_options.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:1: # -*- coding: utf-8 -*-
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:46: def ListAnalysisPlugins(self):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved from psort.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:76: def _CreateOutputModule(self, options):
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: moved from psteal_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:116: missing_parameters =
output_module.GetMissingArguments()
On 2017/07/26 05:00:53, aaronp wrote:
> The changes in this method are good ones, but a good example of things that
> could have been done in a second PR to make this PR easier to review since a
lot
> of the other code in this method was just moved from another method.

Acknowledged.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:135: def _GetOutputModulesInformation(self):
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: Moved from log2timeline_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:147: def ListOutputModules(self):
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: moved from log2timeline_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tool_options.py...
plaso/cli/tool_options.py:173: def _CheckStorageFile(self, storage_file_path):
On 2017/06/29 01:32:22, aaronp wrote:
> Note to self: moved from each of cli/{log2timeline.py,psort_tool.py,psteal.py}

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py
File plaso/cli/tools.py (left):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#oldcode92
plaso/cli/tools.py:92: def _ClearScreen(self):
On 2017/07/14 23:59:52, aaronp wrote:
> On 2017/07/14 00:31:42, aaronp wrote:
> > Note to self: No refs found.
> 
> Actually, this is moved to status_view.py ...

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#oldcod...
plaso/cli/tools.py:174: def _ParseDataLocationOption(self, options):
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self:  Moved to helpers/data_location.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#oldcod...
plaso/cli/tools.py:313: def AddDataLocationOption(self, argument_group):
On 2017/07/14 23:59:52, aaronp wrote:
> Note to self: Moved to data_location.py

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py
File plaso/cli/tools.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#newcode1
plaso/cli/tools.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#newcod...
plaso/cli/tools.py:157: def _ParseLogFileOptions(self, options):
I've added a note to https://github.com/log2timeline/plaso/issues/160

https://codereview.appspot.com/326740043/diff/60001/plaso/cli/tools.py#newcod...
plaso/cli/tools.py:230: def _PromptUserForInput(self, input_text):
Not sure if I fully understand your question, but this function is used under
the hood to ask the user e.g. for which partitions to process, etc.

https://codereview.appspot.com/326740043/diff/60001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/output/manager.py
File plaso/output/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/output/manager.py#n...
plaso/output/manager.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: Reviewed

Done.

https://codereview.appspot.com/326740043/diff/60001/plaso/parsers/manager.py
File plaso/parsers/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/plaso/parsers/manager.py#...
plaso/parsers/manager.py:1: # -*- coding: utf-8 -*-
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/analysis/manager.py
File tests/analysis/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/analysis/manager.py...
tests/analysis/manager.py:1: #!/usr/bin/python
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/analysis_tool.py
File tests/cli/analysis_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/analysis_tool.p...
tests/cli/analysis_tool.py:1: #!/usr/bin/python
On 2017/07/14 23:59:51, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extract_analyze...
File tests/cli/extract_analyze_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extract_analyze...
tests/cli/extract_analyze_tool.py:1: #!/usr/bin/python
On 2017/07/14 23:59:51, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool.py
File tests/cli/extraction_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool...
tests/cli/extraction_tool.py:130:
@shared_test_lib.skipUnlessHasTestFile([u'artifacts'])
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: moved to tests/cli/helpers/analysis_plugins.py

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool.py
File tests/cli/extraction_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/extraction_tool...
tests/cli/extraction_tool.py:1: #!/usr/bin/python
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/analysi...
File tests/cli/helpers/analysis_plugins.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/analysi...
tests/cli/helpers/analysis_plugins.py:1: #!/usr/bin/python
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/analysi...
tests/cli/helpers/analysis_plugins.py:68: # TODO: improve test coverage.
On 2017/07/14 00:31:42, aaronp wrote:
> Maybe add testing '--analysis list' specifically to this TODO.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/artifac...
File tests/cli/helpers/artifact_definitions.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/artifac...
tests/cli/helpers/artifact_definitions.py:1: #!/usr/bin/python
On 2017/07/14 00:31:42, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/artifac...
tests/cli/helpers/artifact_definitions.py:21: _EXPECTED_OUTPUT = u'\n'.join([
could be an option, there is a note in
https://github.com/log2timeline/plaso/issues/160 to look into this. I did not
wanted to cram that into this CL ;)

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/data_lo...
File tests/cli/helpers/data_location.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/data_lo...
tests/cli/helpers/data_location.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/dynamic...
File tests/cli/helpers/dynamic_output.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/dynamic...
tests/cli/helpers/dynamic_output.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/event_f...
File tests/cli/helpers/event_filters.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/event_f...
tests/cli/helpers/event_filters.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/filter_...
File tests/cli/helpers/filter_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/filter_...
tests/cli/helpers/filter_file.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/languag...
File tests/cli/helpers/language.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/languag...
tests/cli/helpers/language.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/manager.py
File tests/cli/helpers/manager.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/manager...
tests/cli/helpers/manager.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
File tests/cli/helpers/output_modules.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
tests/cli/helpers/output_modules.py:1: #!/usr/bin/python
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
tests/cli/helpers/output_modules.py:74: options = cli_test_lib.TestOptions()
On 2017/07/14 23:59:53, aaronp wrote:
> nit: I would separate these into at least two tests.

Acknowledged.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/output_...
tests/cli/helpers/output_modules.py:82: with
self.assertRaises(errors.BadConfigOption):
I'll add some comments for now

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/status_...
File tests/cli/helpers/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/status_...
tests/cli/helpers/status_view.py:1: #!/usr/bin/python
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/storage...
File tests/cli/helpers/storage_file.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/storage...
tests/cli/helpers/storage_file.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/tempora...
File tests/cli/helpers/temporary_directory.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/tempora...
tests/cli/helpers/temporary_directory.py:1: #!/usr/bin/python
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/yara_ru...
File tests/cli/helpers/yara_rules.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/helpers/yara_ru...
tests/cli/helpers/yara_rules.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/hexdump.py
File tests/cli/hexdump.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/hexdump.py#oldc...
tests/cli/hexdump.py:1: #!/usr/bin/python
On 2017/07/14 00:31:41, aaronp wrote:
> Note to self: Reviewed.  Doesn't seem to be used anywhere.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/log2timeline_to...
File tests/cli/log2timeline_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/log2timeline_to...
tests/cli/log2timeline_tool.py:1: #!/usr/bin/python
On 2017/07/26 05:00:53, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py
File tests/cli/psort_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#o...
tests/cli/psort_tool.py:346: def testListAnalysisPlugins(self):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved to test*/tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#o...
tests/cli/psort_tool.py:401: def testListOutputModules(self):
On 2017/07/26 22:06:09, aaronp wrote:
> Note to self: Moved to tests*/tool_options.py

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py
File tests/cli/psort_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#n...
tests/cli/psort_tool.py:1: #!/usr/bin/python
On 2017/07/26 22:06:10, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psort_tool.py#n...
tests/cli/psort_tool.py:118: # TODO: add test for _PrintStatusHeader.
On 2017/07/14 23:59:53, aaronp wrote:
> This went away when it migrated to status_view.py.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psteal_tool.py
File tests/cli/psteal_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psteal_tool.py#...
tests/cli/psteal_tool.py:1: #!/usr/bin/python
On 2017/07/26 22:06:10, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/psteal_tool.py#...
tests/cli/psteal_tool.py:36: test_tool._source_path = u'/test/storage/path'
I've added a reminder to https://github.com/log2timeline/plaso/issues/160

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view.py
File tests/cli/status_view.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view.py#...
tests/cli/status_view.py:1: #!/usr/bin/python
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: Reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view_too...
File tests/cli/status_view_tool.py (left):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/status_view_too...
tests/cli/status_view_tool.py:1: #!/usr/bin/python
On 2017/07/26 05:00:52, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/storage_media_t...
File tests/cli/storage_media_tool.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/storage_media_t...
tests/cli/storage_media_tool.py:1: #!/usr/bin/python
On 2017/07/26 22:06:10, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py
File tests/cli/tool_options.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:1: #!/usr/bin/python
On 2017/07/26 22:06:10, aaronp wrote:
> Note to self: reviewed.

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:16: """Tool to test the analysis plugin options."""
It is not the current convention but style guide is not explicit about it. I opt
something to discuss outside of this CL. Added reminder to:
https://github.com/log2timeline/plaso/issues/160

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:22: def testListAnalysisPlugins(self):
On 2017/07/26 22:06:10, aaronp wrote:
> Note to self: Moved from tests*/psort.py

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:61: def testGetOutputModulesInformation(self):
On 2017/07/26 22:06:10, aaronp wrote:
> Note to self: moved from tests*/log2timeline.py

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tool_options.py...
tests/cli/tool_options.py:72: def testListOutputModules(self):
On 2017/07/26 22:06:10, aaronp wrote:
> Note to self: Moved from tests*/psort_tool.py

Done.

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tools.py
File tests/cli/tools.py (right):

https://codereview.appspot.com/326740043/diff/60001/tests/cli/tools.py#newcode1
tests/cli/tools.py:1: #!/usr/bin/python
On 2017/07/14 23:59:53, aaronp wrote:
> Note to self: Reviewed.

Done.

[Joachim Metz, 2017-07-27 06:07:43]

merged via github due to pylint 1.7 linting conflicts