Skip to content

update parameter option names to be clearer#242

Merged
wjwwood merged 1 commit intomasterfrom
rename_parameter_options
May 29, 2019
Merged

update parameter option names to be clearer#242
wjwwood merged 1 commit intomasterfrom
rename_parameter_options

Conversation

@wjwwood
Copy link
Copy Markdown
Member

@wjwwood wjwwood commented May 28, 2019 •
edited
Loading

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:

@wjwwood
update parameter option names to be clearer
3584887 
Signed-off-by: William Woodall <william@osrfoundation.org>
@wjwwood wjwwood requested review from chapulina and ivanpauno May 28, 2019 21:58
@wjwwood wjwwood self-assigned this May 28, 2019
This was referenced May 28, 2019
Merged
Merged
Merged
Merged
@wjwwood
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Reviewers

@ivanpauno ivanpauno ivanpauno approved these changes

+1 more reviewer

@chapulina chapulina chapulina approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@wjwwood wjwwood

Labels

in review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wjwwood @chapulina @ivanpauno @nuclearsandwich