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

feat(lanelet2_extension): update documentation for bus_stop_area to be introduced in format_version2 #21

Merged
merged 6 commits into from
Aug 26, 2024

Conversation

soblin
Copy link
Collaborator

@soblin soblin commented Aug 19, 2024

Description

add the documentation for description and specification for bus_stop_area and bicycle_lane

https://github.com/orgs/autowarefoundation/discussions/5097

Related links

https://github.com/orgs/autowarefoundation/discussions/5097

Tests performed

documentation PR

Notes for reviewers

Interface changes

added new map primitives

Effects on system behavior

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.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

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.
  • The PR is ready for merge.

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

Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.84%. Comparing base (1d3219a) to head (0d2c1c3).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #21   +/-   ##
=======================================
  Coverage   15.84%   15.84%           
=======================================
  Files          40       40           
  Lines        2765     2765           
  Branches     1348     1348           
=======================================
  Hits          438      438           
  Misses       2239     2239           
  Partials       88       88           
Flag Coverage Δ *Carryforward flag
differential 15.84% <ø> (-0.91%) ⬇️
total 15.84% <ø> (ø) Carriedforward from c8f3596

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

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

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

You can separate this PR into following 3 PRs to review easily.

  1. Implement BusStopArea class and modify CMakeLists.txt
  2. Update format version namespace
  3. Update documentation

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

@soblin
Copy link
Collaborator Author

soblin commented Aug 19, 2024

@youtalk All right, I will update the document at first in this PR !

Signed-off-by: Mamoru Sobue <[email protected]>
@soblin soblin changed the title feat(lanelet2_extension): format version2, introduce bus_stop_area and bicycle_lane feat(lanelet2_extension): update documentation for bus_stop_area and bicycle_lane to be introduced in format_version2 Aug 19, 2024
@youtalk
Copy link
Member

youtalk commented Aug 20, 2024

There are 5 typos. You fixed only one of them.

autoware_lanelet2_extension/docs/extra_lanelet_subtypes.md:53:8 Unknown word (bicicyle)
autoware_lanelet2_extension/docs/extra_lanelet_subtypes.md:54:172 Unknown word (formart)
autoware_lanelet2_extension/docs/extra_lanelet_subtypes.md:55:143 Unknown word (dicussion)
autoware_lanelet2_extension/docs/extra_regulatory_elements.md:314:5 Unknown word (eample)
autoware_lanelet2_extension/docs/lanelet2_format_extension.md:29:328 Unknown word (Lanlet)

Signed-off-by: Mamoru Sobue <[email protected]>
@soblin
Copy link
Collaborator Author

soblin commented Aug 20, 2024

typos have been fixed.

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

Ah, you made a force push. I just saw the commit 38903c9.

The documentation update is the 3rd step. I will review again after 1st and 2nd steps merged.

You can separate this PR into following 3 PRs to review easily.

  1. Implement BusStopArea class and modify CMakeLists.txt
  2. Update format version namespace
  3. Update documentation

@soblin
Copy link
Collaborator Author

soblin commented Aug 20, 2024

OK, then I will first make this PR ready:

It adds BusStopArea implementation in format_v2, but format_v1 still remains the default.

Then this PR releases format_v2 and I'm running T4 E2E test now.

I prefer bringing #23 PR last because it upgrades the API version.

@mitsudome-r
Copy link
Member

Is it possible to split the PR to addition of bus_stop_area and bicycle_lane with the rest of refactoring of the documentation?
I'm sorry for splitting the PRs into so many pieces, but I would like to make sure that each modification in different contexts are merged through different PRs.

Signed-off-by: Mamoru Sobue <[email protected]>
Signed-off-by: Mamoru Sobue <[email protected]>
Signed-off-by: Mamoru Sobue <[email protected]>
@soblin soblin changed the title feat(lanelet2_extension): update documentation for bus_stop_area and bicycle_lane to be introduced in format_version2 feat(lanelet2_extension): update documentation for bicycle_lane to be introduced in format_version2 Aug 23, 2024
@soblin soblin changed the title feat(lanelet2_extension): update documentation for bicycle_lane to be introduced in format_version2 feat(lanelet2_extension): update documentation for bus_stop_area to be introduced in format_version2 Aug 23, 2024
@soblin
Copy link
Collaborator Author

soblin commented Aug 23, 2024

@youtalk With #22 merged, can you approve this PR ?

Copy link
Member

@youtalk youtalk 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 your cooperation to split PRs. It's very easy to review them.
LGTM

@soblin soblin merged commit 880edbb into main Aug 26, 2024
22 of 23 checks passed
@soblin soblin deleted the feat/format-v2 branch August 26, 2024 01:59
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.

3 participants