Skip to content

Parameters roundup#4

Merged
esteve merged 1 commit intomasterfrom
parameters-roundup
Jun 16, 2015
Merged

Parameters roundup#4
esteve merged 1 commit intomasterfrom
parameters-roundup

Conversation

@esteve
Copy link
Copy Markdown
Member

@esteve esteve commented Jun 15, 2015

This should fix any inconsistencies and reflect the feedback from ros2/ros2#46

@dirk-thomas @jacquelinekay @tfoote @wjwwood please review

Connects to ros2/ros2#46

@esteve esteve added the in progress Actively being worked on (Kanban column) label Jun 15, 2015
@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Jun 15, 2015

+1

1 similar comment
@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Jun 15, 2015

+1

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Jun 15, 2015

A couple minor changes to the comment in the message files:

  • The comment in the renamed ParameterEventDescriptors file needs to be updated to descriptors
  • The comment for the reply in SetParameters.srv seems to not make sense anymore

The following comments are from the referenced ticket:

  • DescribeParameters should explicitly mention same length (beside same order)
  • ParameterEvent: mention constraints about entries with same names in different fields as discussed (same for ParameterDescriptor)

@esteve esteve force-pushed the parameters-roundup branch from 01acfd1 to fdadefe Compare June 15, 2015 22:38
@esteve
Copy link
Copy Markdown
Member Author

esteve commented Jun 15, 2015

@dirk-thomas done.

This was referenced Jun 15, 2015
Merged
Merged
@esteve esteve self-assigned this Jun 16, 2015
@esteve esteve force-pushed the parameters-roundup branch from fdadefe to 0672fd1 Compare June 16, 2015 01:36
@esteve
Copy link
Copy Markdown
Member Author

esteve commented Jun 16, 2015

@dirk-thomas fixed.

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Jun 16, 2015

Beside the last micro comment: +1

@esteve esteve force-pushed the parameters-roundup branch from 0672fd1 to f00fe22 Compare June 16, 2015 18:04
esteve added a commit that referenced this pull request Jun 16, 2015
@esteve
Merge pull request #4 from ros2/parameters-roundup
1c3a468 
Parameters roundup
@esteve esteve merged commit 1c3a468 into master Jun 16, 2015
@esteve esteve removed the in progress Actively being worked on (Kanban column) label Jun 16, 2015
@esteve esteve deleted the parameters-roundup branch June 16, 2015 18:05
@cevans87 cevans87 mentioned this pull request Feb 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@esteve esteve

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@esteve @wjwwood @tfoote @dirk-thomas