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.

Sign up for GitHub

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

Jump to bottom

Add parsing method for dirname substitution #273

Merged
merged 4 commits into from
Jul 16, 2019
Merged

Add parsing method for dirname substitution #273

merged 4 commits into from
Jul 16, 2019

Conversation

ivanpauno
Copy link
Member

Also, corrected problem for parsing substitutions with no arguments.

@ivanpauno
Add parsing method for dirname substitution
30ebe18 
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno added enhancement New feature or request in review Waiting for review (Kanban column) labels Jul 16, 2019
@ivanpauno ivanpauno requested a review from hidmic July 16, 2019 13:53
@ivanpauno ivanpauno self-assigned this Jul 16, 2019
@ivanpauno ivanpauno added this to Proposed in Dashing Patch Release 2 via automation Jul 16, 2019
@ivanpauno ivanpauno mentioned this pull request Jul 16, 2019
Merged
hidmic
hidmic requested changes Jul 16, 2019
View reviewed changes
launch/launch/frontend/expose.py Outdated Show resolved Hide resolved
launch/launch/substitutions/this_launch_file_dir.py Show resolved Hide resolved
@ivanpauno
Nicer code
816b94a 
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
hidmic
hidmic approved these changes Jul 16, 2019
View reviewed changes
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM

launch/launch/substitutions/this_launch_file_dir.py Outdated Show resolved Hide resolved
@ivanpauno
Address review comment
dc75c23 
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Member Author

CI (up to launch, launch_xml, launch_yaml, launch_ros, test_launch_ros, only fastrtps):

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

wjwwood
wjwwood reviewed Jul 16, 2019
View reviewed changes
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.

One thing I think should be changed, but otherwise lgtm.

launch/launch/frontend/expose.py Outdated Show resolved Hide resolved
@ivanpauno
Address PR comment
d10d64b 
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Member Author

CI (up to launch, launch_xml, launch_yaml, launch_ros, test_launch_ros, only fastrtps):

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

@ivanpauno ivanpauno merged commit 43af71e into master Jul 16, 2019
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/dirname-substitution-parsing-method branch July 16, 2019 18:49
@ivanpauno ivanpauno removed this from Proposed in Dashing Patch Release 2 Jul 18, 2019
@ivanpauno ivanpauno added this to In progress in Eloquent via automation Jul 18, 2019
@nuclearsandwich nuclearsandwich moved this from In progress to Done in Eloquent Aug 8, 2019
piraka9011 pushed a commit to aws-ros-dev/launch that referenced this pull request Aug 16, 2019
@ivanpauno @piraka9011
Add parsing method for dirname substitution (ros2#273)
f7ec218 
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood approved these changes

@hidmic hidmic hidmic approved these changes

Assignees

@ivanpauno ivanpauno

Labels
enhancement New feature or request in review Waiting for review (Kanban column)
Projects
No open projects
Eloquent
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ivanpauno @wjwwood @hidmic