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

build(yabloc_pose_initializer): remove downloading logic from CMake #4905

Merged
merged 14 commits into from
Sep 25, 2023

Conversation

lexavtanke
Copy link
Contributor

@lexavtanke lexavtanke commented Sep 6, 2023

Description

See #3137

According to discussion above remove downloading logic from building stage. For now it will be possible to download model files either by ansible or manually as explained in this PR

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: Alexey Panferov <lexavtanke@gmail.com>
@github-actions github-actions bot added the component:localization Vehicle's position determination in its environment. (auto-assigned) label Sep 6, 2023
Copy link
Contributor

@esteve esteve left a comment

Choose a reason for hiding this comment

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

@lexavtanke changes look good, thanks. Not sure if this PR is ready for review. Just some feedback, could you update the title and the commit message so that they follow the Autoware conventions? That is, "build(yabloc_pose_initializer): remove downloading logic from CMake" Thanks.

@lexavtanke lexavtanke changed the title remove downloading logic from Cmake build(yabloc_pose_initializer): remove downloading logic from CMake Sep 7, 2023
@lexavtanke lexavtanke added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Sep 8, 2023
@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.13% 🎉

Comparison is base (71da68c) 14.99% compared to head (379ec5a) 15.12%.
Report is 1 commits behind head on main.

❗ Current head 379ec5a differs from pull request most recent head d230cb9. Consider uploading reports for the commit d230cb9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4905      +/-   ##
==========================================
+ Coverage   14.99%   15.12%   +0.13%     
==========================================
  Files        1608     1577      -31     
  Lines      111834   107182    -4652     
  Branches    34693    32676    -2017     
==========================================
- Hits        16764    16215     -549     
+ Misses      76336    73206    -3130     
+ Partials    18734    17761     -973     
Flag Coverage Δ *Carryforward flag
differential 0.00% <ø> (?)
total 15.13% <ø> (+0.14%) ⬆️ Carriedforward from c8055f0

*This pull request uses carry forward flags. Click here to find out more.

see 258 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@esteve
Copy link
Contributor

esteve commented Sep 22, 2023

@lexavtanke is this PR now ready for review?

@lexavtanke
Copy link
Contributor Author

@esteve Yes it is ready, but it should be merged after 3855 in autoware. As artifacts for yabloc should be extracted before usage.
So I keep it draft for now.

@esteve esteve marked this pull request as ready for review September 22, 2023 14:26
@esteve esteve enabled auto-merge (squash) September 22, 2023 14:26
@esteve
Copy link
Contributor

esteve commented Sep 22, 2023

@KYabuuchi @Motsu-san could you have a look at this PR when you have a moment? Thanks.

@KYabuuchi
Copy link
Contributor

KYabuuchi commented Sep 24, 2023

@lexavtanke Thank you for creating the PR.
There are two other files that explain the old download instructions, so please fix those as well. 🙏
Specifically, please remove or update the text outlined in red below.

  • README.md
    image

  • src/camera/semantic_segmentation.cpp
    image

@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Sep 25, 2023
pre-commit-ci bot and others added 3 commits September 25, 2023 14:00
Signed-off-by: Alexey Panferov <lexavtanke@gmail.com>
Signed-off-by: Alexey Panferov <lexavtanke@gmail.com>
@lexavtanke
Copy link
Contributor Author

@KYabuuchi Thank you for your feedback. I've made updates to these files. Can you please review the PR then you will have time?

Copy link
Contributor

@KYabuuchi KYabuuchi left a comment

Choose a reason for hiding this comment

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

Thank you for adding clear download instructions. This PR looks good to me 👍

@esteve esteve merged commit 46f871f into autowarefoundation:main Sep 25, 2023
16 of 18 checks passed
@lexavtanke lexavtanke deleted the remove_download_yabloc branch September 26, 2023 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:localization Vehicle's position determination in its environment. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants