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

Warn unused #327

Merged
merged 7 commits into from
May 26, 2017
Merged

Warn unused #327

merged 7 commits into from
May 26, 2017

Conversation

Karsten1987
Copy link
Contributor

fixes flakiness

@Karsten1987 Karsten1987 added the in review Waiting for review (Kanban column) label May 23, 2017
@Karsten1987 Karsten1987 self-assigned this May 23, 2017
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm (other than one comment about the copyright year)

@@ -0,0 +1,55 @@
// Copyright 2015 Open Source Robotics Foundation, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

2017

EXPECT_EQ(3, c_state2.id());
EXPECT_FALSE(c_state2.label().empty());
ASSERT_STREQ("my_c_state2", c_state2.label().c_str());
// introduces flakiness
Copy link
Member

Choose a reason for hiding this comment

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

Is this ticketed somewhere ? or should we add a TODO here to remember that there is a commented out test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The commented out test is on purpose. When compiling with actual memory sanitizer, this last tests fails and thus produces a flaky test.

@mikaelarguedas
Copy link
Member

BTW: really like the description text of this PR 😄

@Karsten1987 Karsten1987 merged commit 708903e into master May 26, 2017
@Karsten1987 Karsten1987 deleted the warn_unused branch May 26, 2017 02:52
@Karsten1987 Karsten1987 removed the in review Waiting for review (Kanban column) label May 26, 2017
nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022
* Adding new cli parameters for configuring the logging.

cr https://code.amazon.com/reviews/CR-3728503

* Moving the external logger library from rcutils into rcl. Also automatic uncrustify.

* Fixing issue with rebase

* Temporarily switching the default to noop logger ext_lib while system dependency issue is solved for Windows/Mac.

* Fixing the format2/3 xmllint issue.

* style changes, and one actual fix

Signed-off-by: William Woodall <william@osrfoundation.org>

* rename rc_logging* to rcl_logging*

Signed-off-by: William Woodall <william@osrfoundation.org>

* Expand rcl_logging_configure API with allocator

This is to make sure that this API is complete, and we can merge rcl#350 later without breaking the API at a later date.

* update logging.c with new use of allocator in API

* update init.c with new API changes

* Switching back to va_list for output handlers.

* address review comments

Signed-off-by: William Woodall <william@osrfoundation.org>
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.

3 participants