return an error status on error in rosbag#1257
Conversation
|
Hey there.. New here and interested in getting this fix into kinetic... |
|
@phil-marble it typically takes a few days for someone to have time to review and either merge this or give you feedback on what needs to change. After merging, it will take a few weeks to a month to get into the public |
|
Thanks @wjwwood ! It's not a huge rush, just wondering about process. I submitted this PR against the default branch ( |
wjwwood
left a comment
There was a problem hiding this comment.
The change looks reasonable to me.
I only identified this line which might also benefit from using sys.exit instead of return:
I think sys.exit make more sense because of how the "cmd" functions are executed:
While this line (and few others that follow it) could (probably should) be converted to use sys.exit rather than exit:
After a quick scan I didn't see anything else that needed doing related to returning an error code when there's a problem.
I'll leave it up to one of the actual maintainers to review and approve this change however.
|
Opening against lunar-devel (or the latest branch) is the right thing to do. The maintainers will back port changes at their discretion or your request. |
|
|
||
| print('Try running \'rosbag check\' to create the necessary rule files or run \'rosbag fix\' with the \'--force\' option.') | ||
| os.remove(outname) | ||
| sys.exit(1) |
There was a problem hiding this comment.
The indentation of this line uses a tab which isn't allowed in Python 3. Please change this to spaces.
|
👍 |
I added some sys.exit(1)'s to rosbag commands to help with scripting. Specifically, I expected a call to `rosbag info test.bag.active` to return an error when the bag needed re-indexing.
c7aee30 to
0df9572
Compare
|
removed the tab |
|
Thank you. |
I added some sys.exit(1)'s to rosbag commands to help with scripting. Specifically, I expected a call to `rosbag info test.bag.active` to return an error when the bag needed re-indexing.
I added some sys.exit(1)'s to rosbag commands to help with scripting.
Specifically, I expected a call to
rosbag info test.bag.activetoreturn an error when the bag needed re-indexing.
fixes #1256