-
Notifications
You must be signed in to change notification settings - Fork 664
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
refactor(autoware_object_merger): move headers to src and rename package #7804
refactor(autoware_object_merger): move headers to src and rename package #7804
Conversation
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
a8d166f
to
b28c98d
Compare
@technolojin I've submitted this PR as a follow-up to #7642 to update the package name and move the headers, let me know if the changes are ok. Thanks. |
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.
@technolojin I've submitted this PR as a follow-up to #7642 to update the package name and move the headers, let me know if the changes are ok. Thanks.
Thank you for your work.
-
package renaming
I forgot to explain my strategy that the package names in /perception will be changed later in a concentrated time to minimize impact of other linked repositories and systems rely on.
However, this package name seems to be fine to be changed (considering the package name is used within autoware_universe). -
moving headers
I am considering the association to be shared in the future. this association part is the same asmulti_object_tracker
,radar_object_tracker
, andtracking_object_merger
.
Let me do the re-organization later.
0333cdf
to
4aa2a5f
Compare
Pull request was converted to draft
Signed-off-by: Esteve Fernandez <[email protected]>
7fdac87
to
948b30f
Compare
@technolojin I've moved the header files to a public directory, can you have a look at the changes when you have a moment? Thanks. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7804 +/- ##
==========================================
- Coverage 28.95% 28.91% -0.05%
==========================================
Files 1606 1611 +5
Lines 117598 117765 +167
Branches 50597 50630 +33
==========================================
Hits 34046 34046
- Misses 74299 74466 +167
Partials 9253 9253
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@yukkysaito @YoshiRi @miursh can you have a look at this PR? Thanks. |
@esteve Thank you for the reminder. |
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.
Can you fix the launcher?
$(find-pkg-share object_merger) -> $(find-pkg-share autoware_object_merger)
perception/autoware_object_merger/launch/object_association_merger.launch.xml
Outdated
Show resolved
Hide resolved
Signed-off-by: Taekjin LEE <[email protected]>
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.
I fixed the launchers.
…age (autowarefoundation#7804) Signed-off-by: Esteve Fernandez <[email protected]> Signed-off-by: Y.Hisaki <[email protected]>
…age (autowarefoundation#7804) Signed-off-by: Esteve Fernandez <[email protected]>
…age (autowarefoundation#7804) Signed-off-by: Esteve Fernandez <[email protected]> Signed-off-by: chtseng <[email protected]>
…age (autowarefoundation#7804) Signed-off-by: Esteve Fernandez <[email protected]>
Description
Move headers to src and rename package
Related links
Parent Issue:
How was this PR tested?
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.