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

Towards Quality Level 1 - Added common_linters and fixed tests #43

Merged
merged 11 commits into from
Apr 22, 2020
Merged

Towards Quality Level 1 - Added common_linters and fixed tests #43

merged 11 commits into from
Apr 22, 2020

Conversation

ahcorde
Copy link

@ahcorde ahcorde commented Apr 20, 2020

  • Added common_linters
  • Fixed C++ tests
  • Fixed Python API
  • Fixed Python tests
  • Fixed license headers

Signed-off-by: ahcorde ahcorde@gmail.com

@ahcorde
Added common_linters and fixed tests
a0b44a5 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde self-assigned this Apr 20, 2020
clalancette
clalancette requested changes Apr 20, 2020
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

A few things to fix in here, and some questions.

resource_retriever/CMakeLists.txt Outdated Show resolved Hide resolved
resource_retriever/test/CMakeLists.txt Outdated Show resolved Hide resolved
resource_retriever/test/test.cpp Outdated Show resolved Hide resolved
resource_retriever/test/test.cpp Outdated Show resolved Hide resolved
resource_retriever/test/test.cpp Outdated Show resolved Hide resolved
resource_retriever/test/test.cpp Outdated Show resolved Hide resolved
ahcorde and others added 6 commits April 20, 2020 16:06
@ahcorde
Removed test/CMakeLists.txt
edd0621 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde @clalancette
Update resource_retriever/test/test.cpp
53bc036 
Co-Authored-By: Chris Lalancette <clalancette@openrobotics.org>
@ahcorde
Const exceptions
071a033 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Added message when throws an exception
3b5aa92 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
combine file plut error
09863db 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Removed trace when the test is right
d8e3f94 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Installing only the test.txt file when BUILD_TESTING is active
63fc6e7 
Signed-off-by: ahcorde <ahcorde@gmail.com>
clalancette
clalancette approved these changes Apr 21, 2020
View reviewed changes
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks OK to me now, but I'd like to get a review from @sloretz before merging.

sloretz
sloretz reviewed Apr 21, 2020
View reviewed changes
Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

LGTM with one nit and green CI

resource_retriever/CMakeLists.txt Outdated Show resolved Hide resolved
@ahcorde
no need to check for target in test
4f3ec2e 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Author

ahcorde commented Apr 21, 2020

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

@ahcorde
Attemp to fix windows path
f7dea60 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Author

ahcorde commented Apr 22, 2020 •
edited
Loading

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

@ahcorde
Fixed warning on Windows
fa481b1 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Author

ahcorde commented Apr 22, 2020

Fixing warning on windows

  • Windows Build Status

@ahcorde ahcorde merged commit 9f02908 into ros2 Apr 22, 2020
@clalancette clalancette deleted the ahcorde/test/linters_and_tests branch June 14, 2021 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sloretz sloretz sloretz left review comments

@clalancette clalancette clalancette approved these changes

Assignees

@ahcorde ahcorde

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ahcorde @clalancette @sloretz