Skip to content
This repository has been archived by the owner on Dec 13, 2021. It is now read-only.

cli: -stdout option for process inspector #154

Merged
merged 1 commit into from
May 27, 2016
Merged

cli: -stdout option for process inspector #154

merged 1 commit into from
May 27, 2016

Conversation

mitake
Copy link
Contributor

@mitake mitake commented May 27, 2016

This commit adds a new option -stdout to process inspector (-cmd
mode). A given path will be used for storing stdout of the target
process. If the path isn't given, os.Stdout will be used.

Fixes #152

@mitake
Copy link
Contributor Author

mitake commented May 27, 2016

PTAL @AkihiroSuda

@@ -44,6 +46,7 @@ func init() {
procFlagset.IntVar(&_procFlags.RootPID, "pid", -1, "PID for the target process tree")
procFlagset.DurationVar(&_procFlags.WatchInterval, "watch-interval", 1*time.Second, "Watching interval")
procFlagset.StringVar(&_procFlags.Cmd, "cmd", "", "Command for target process")
procFlagset.StringVar(&_procFlags.Stdout, "stdout", "", "Stdout for target process (used if -cmd option is given)")
Copy link
Member

Choose a reason for hiding this comment

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

I think -cmd-stdout is much more descriptive.
If you feel -cmd-stdout is too long, I'm +1 for keeping -stdout

Copy link
Member

Choose a reason for hiding this comment

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

We need -stderr as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added -stderr. I think -cmd-stdout is too long because the option is only used with -cmd and it is described in the help message.

@AkihiroSuda
Copy link
Member

Thank you a lot for working on this!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 41.525% when pulling 33c9856 on mitake:stdout into 89dea61 on osrg:master.

@mitake
Copy link
Contributor Author

mitake commented May 27, 2016

@AkihiroSuda thanks for review, updated for the above comment, PTAL.

@@ -44,6 +47,8 @@ func init() {
procFlagset.IntVar(&_procFlags.RootPID, "pid", -1, "PID for the target process tree")
procFlagset.DurationVar(&_procFlags.WatchInterval, "watch-interval", 1*time.Second, "Watching interval")
procFlagset.StringVar(&_procFlags.Cmd, "cmd", "", "Command for target process")
procFlagset.StringVar(&_procFlags.Stdout, "stdout", "", "Stdout for target process (used if -cmd option is given)")
procFlagset.StringVar(&_procFlags.Stderr, "stderr", "", "Stdout for target process (used if -cmd option is given)")
Copy link
Member

Choose a reason for hiding this comment

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

the help string is still "Stdout..""

@AkihiroSuda
Copy link
Member

@mitake Thank you, please fix the small nit about the help string?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 41.376% when pulling f26eef7 on mitake:stdout into 89dea61 on osrg:master.

This commit adds a new option -stdout to process inspector (-cmd
mode). A given path will be used for storing stdout of the target
process. If the path isn't given, os.Stdout will be used. -stderr does
same thing for os.Stderr.
@mitake
Copy link
Contributor Author

mitake commented May 27, 2016

@AkihiroSuda ah sorry for that, fixed it, PTAL.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 41.376% when pulling 85a9761 on mitake:stdout into 89dea61 on osrg:master.

@AkihiroSuda AkihiroSuda merged commit d56ed94 into osrg:master May 27, 2016
@AkihiroSuda
Copy link
Member

Thank you, merged

@mitake mitake deleted the stdout branch May 27, 2016 06:42
AkihiroSuda added a commit that referenced this pull request Sep 5, 2016
Release Note: http://osrg.github.io/namazu/post/release-0-2-1/

Changes from v0.2.0:

 * #167, #168, #169, #170: doc: miscellaneous improvements
 * #166: vendor go packages
 * #163: *: bump up Go to 1.7
 * #162: container: add support for inspectors/fs
 * #160: inspectors/fs: implement Fsync
 * #158, #159: inspectors/fs: improved CLI (thank you @v01dstar !)
 * #156: inspectors/proc: improved error handling
 * #154, #155: inspectors/proc: improved CLI

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
@AkihiroSuda AkihiroSuda mentioned this pull request Sep 5, 2016
AkihiroSuda added a commit that referenced this pull request Sep 5, 2016
Release Note: http://osrg.github.io/namazu/post/release-0-2-1/

Changes from v0.2.0:

 * #167, #168, #169, #170: doc: miscellaneous improvements
 * #166: vendor go packages
 * #163: *: bump up Go to 1.7
 * #162: container: add support for inspectors/fs
 * #160: inspectors/fs: implement Fsync
 * #158, #159: inspectors/fs: improved CLI (thank you @v01dstar !)
 * #156: inspectors/proc: improved error handling
 * #154, #155: inspectors/proc: improved CLI

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants