-
Notifications
You must be signed in to change notification settings - Fork 300
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
Migrate launch tests to new launch_testing features & API #179
Conversation
Please use a separate commit (which isn't being squashed when merging later) to rename existing files (instead of deleting one and creating another with very similar content). |
Hmm, it is true that the two current tests were based upon an existing one that was later deleted. But this isn't pure renaming, the original test was split in two. Would you rather have the original test renamed to one of the current ones? |
I just want the history to be preserved. If you want to use a different filename then you should rename the file in a separate commit without making any changes to it. Follow up commits can then change the content in any way. |
98f069f
to
a96ee95
Compare
Done, sorry about the force push. Github's UI shows the original file as deleted though. |
The blame view can now track changes from the past: https://github.com/ros2/ros1_bridge/blame/a96ee952c63c0975887c09057b1b6e062c881951/test/test_topics_across_dynamic_bridge.py.in |
Looking at the diff now it seems that the service file is also derived from the original file (I though it was a new addition). In that case it would be good to copy the file after renaming it and before making the changes - again to be able to track the changes of the content. |
a96ee95
to
835ab7d
Compare
Done @dirk-thomas ! |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
835ab7d
to
1c843f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, pending CI
@jacobperron @dirk-thomas mind to approve with powers? @ivanpauno lacks them. |
* Update after launch_testing features becoming legacy. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Rename launch based tests in preparation for a refactor. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Migrate tests to new launch_testing API. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> Signed-off-by: Dhananjay Sathe <dhananjay.sathe@rapyuta-robotics.com>
Connected to ros2/launch#208.