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

update parameter option names to be clearer #242

Merged
merged 1 commit into from
May 29, 2019

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented May 28, 2019

This is pull request (and the connected pull requests) change the name of two parameter related options:

  • initial_parameter -> parameter_overrides
    • to reflect the fact that by default these parameter values are used only to override the initial value parameters get after being declared or set for the first time
  • automatically_declare_initial_parameters -> automatically_declare_parameters_from_overrides
    • to reflect the fact that it uses the overrides to declare the parameters

This is my proposed solution, based on feedback in ros2/rclcpp#730.

Connected prs:

Signed-off-by: William Woodall <william@osrfoundation.org>
@wjwwood
Copy link
Member Author

wjwwood commented May 28, 2019

Speculative CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood
Copy link
Member Author

wjwwood commented May 29, 2019

New CI after iterating:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood
Copy link
Member Author

wjwwood commented May 29, 2019

I fixed the flake8 warning in rclpy which was the only related test failure for CI. And the pr job on the rclpy pr checked that it was fixed and I tested it locally too.

I'll run CI just up to rclpy on Linux to be sure, but I'll merge after that if there is no more feedback:

  • Linux Build Status

@wjwwood wjwwood merged commit 0f9441c into master May 29, 2019
@delete-merged-branch delete-merged-branch bot deleted the rename_parameter_options branch May 29, 2019 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants