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

reuse camera info in tests, rather than redfining in each method #38

Merged
merged 5 commits into from
Aug 25, 2022

Conversation

ijnek
Copy link
Member

@ijnek ijnek commented Aug 7, 2022

This makes the tests a bit shorter and easier to understand.
I think the tests should pass once #37 is merged.

@ijnek
Copy link
Member Author

ijnek commented Aug 20, 2022

Waiting on an alternative PR for #37 by @Flova before merging these changes in, because the fixes required to get CI passing for this PR will cause a lot of merge conflicts for @Flova on his branch.

Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
@ijnek
Copy link
Member Author

ijnek commented Aug 25, 2022

Waiting on an alternative PR for #37 by @Flova before merging these changes in, because the fixes required to get CI passing for this PR will cause a lot of merge conflicts for @Flova on his branch.

I said this in my previous comment, but I've changed my mind. I'm going to merge this to get it out of the way. I don't think the merge conflicts are going to be bad.

…e tests only pass when there is an invalid camera info.

Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
@ijnek ijnek merged commit dda10d5 into ros-sports:rolling Aug 25, 2022
@ijnek ijnek deleted the ijnek-reuse-cam-info branch August 25, 2022 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants