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

[roslaunch-check] Use roslaunch.core.printerrlog for printing error message. #1193

Merged
merged 1 commit into from
Dec 19, 2017

Conversation

130s
Copy link
Member

@130s 130s commented Oct 18, 2017

Errors are printed without much stress, which makes hard to find the specific error occurred.

…essage.

Errors are printed without much stress, which makes hard to find the specific error occurred.
@130s 130s force-pushed the l/roslaunch-check_color branch from dc554ed to d182f42 Compare October 18, 2017 01:50
@@ -116,7 +117,7 @@ if __name__ == '__main__':
message = "roslaunch check [%s] failed"%(roslaunch_path)
f.write(rosunit.junitxml.test_failure_junit_xml(test_name, message, stdout=error_msg,
class_name="roslaunch.RoslaunchCheck", testcase_name="%s_%s" % (pkg, outname)))
print("wrote test file to [%s]"%test_file)
roslaunch.core.printerrlog("wrote test file to [%s]"%test_file)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to be an error message?

Copy link
Member Author

Choose a reason for hiding this comment

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

This if block is when error occurred. To highlight the file path this change is still useful.

@dirk-thomas
Copy link
Member

Thank you for the patch.

@dirk-thomas dirk-thomas merged commit 2e0a18f into ros:lunar-devel Dec 19, 2017
@130s 130s deleted the l/roslaunch-check_color branch December 19, 2017 14:55
@130s
Copy link
Member Author

130s commented Dec 21, 2017

Thank you for the review @dirk-thomas.
Just wondering if there will be another batch backport like #1008 soon for older supported branches and if this PR will be considered?

@dirk-thomas
Copy link
Member

There will likely be another Lunar release in the near term and after that a batch backport to Kinetic. All changes will be considered - the usual rational applies if a change should be backported. While this patch has little impact and wouldn't be necessary it also poses very little risk. Will see when doing the backporting.

@@ -219,10 +219,10 @@ def check_roslaunch(f, use_test_depends=False):
print(e, file=sys.stderr)
miss_all = True
if miss_all:
print("Missing package dependencies: %s/package.xml: %s"%(pkg, ', '.join(miss)), file=sys.stderr)
roslaunch.core.printerrlog("Missing package dependencies: %s/package.xml: %s"%(pkg, ', '.join(miss)), file=sys.stderr)
Copy link
Member

Choose a reason for hiding this comment

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

The new function doesn't support the file keyword argument. See #1317.

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.

2 participants