Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add output-json option #23

Merged
merged 3 commits into from
Aug 30, 2017
Merged

Add output-json option #23

merged 3 commits into from
Aug 30, 2017

Conversation

giginet
Copy link
Owner

@giginet giginet commented Aug 11, 2017

closes #20

Add command line option --output-path. You can specify the path indicates output path for JSONReporter.

$ xcprofiler --output-path "result.json" MyApp

@giginet giginet self-assigned this Aug 11, 2017
@giginet giginet requested a review from timakin August 11, 2017 12:42
@@ -28,6 +28,7 @@ def execute(args)
opts.on("--derived-data-path", String, "Root path of DerivedData") { |v| options.derived_data_path = v }
opts.on("-t", "--truncate-at [TRUNCATE_AT]", Integer, "Truncate the method name with specified length") { |v| options.truncate_at = v }
opts.on("--[no-]unique", "Reject duplicated location results or not") { |v| options.unique = v }
opts.on("--output-path [PATH]", String, "File path to output reporters' result") { |v| options.output_path = v }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be -o and --output.
At least -path is needless.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-o is already used. So I renamed to --output

if options[:output_path]
json_args = options.dup
json_args[:output_path] = options[:output_path]
reporters << JSONReporter.new(json_args)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other style? Only JSON?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, only JSON

@timakin
Copy link
Collaborator

timakin commented Aug 12, 2017

Is there no test? Maybe you could add the test that generates temporary output JSON and has a validation of regex-match.

@giginet
Copy link
Owner Author

giginet commented Aug 29, 2017

There are no tests parsing options. I'm going to add them later.

@giginet
Copy link
Owner Author

giginet commented Aug 29, 2017

@timakin I fixed.

@timakin
Copy link
Collaborator

timakin commented Aug 30, 2017

LGTM

@giginet
Copy link
Owner Author

giginet commented Aug 30, 2017

❤️

@giginet giginet merged commit e046ef3 into master Aug 30, 2017
@giginet giginet deleted the add-output-json-option branch August 30, 2017 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command line option for output format
2 participants