-
-
Notifications
You must be signed in to change notification settings - Fork 34
Pumpup to auckland #255
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
Pumpup to auckland #255
Conversation
WalkthroughUpdates workshop locale and configuration from Belém (2024) to Auckland (2025); adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer
participant GitHubActions as "Locale CI"
participant Apt as "apt-get"
participant CMake as "CMake"
participant AddCmd as "add_custom_command"
participant Shell as "bash / psql"
Developer->>GitHubActions: push PR / trigger workflow
GitHubActions->>Apt: apt-get install (includes gettext)
Developer->>CMake: configure / build
CMake->>AddCmd: evaluate add_custom_command blocks
note right of AddCmd #fff4e6: Several DEPENDS removed\n(build may no longer be triggered by those files)
AddCmd->>Shell: run COMMAND (bash/psql ... .sql)
Shell-->>AddCmd: produce byproducts (data.txt, etc.)
AddCmd-->>CMake: command complete
CMake-->>Developer: build finished
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/scripts/un_sdg/sdg3/CMakeLists.txt (1)
48-70: Add explicit DEPENDS forall_exercises_sdg3.sql
Without an explicitDEPENDS, CMake won’t rerun the custom command when that SQL file changes—addDEPENDS all_exercises_sdg3.sqlto theadd_custom_command.locale/en/LC_MESSAGES/basic/graphs.po (1)
1-6: Update project metadata to Auckland 3.1.0-dev.Header still references Belém/3.0; please align city/version to this PR’s scope.
Apply:
-# Belém package. +# Auckland package. @@ -"Project-Id-Version: Workshop FOSS4G Belém 3.0\n" +"Project-Id-Version: Workshop FOSS4G Auckland 3.1.0-dev\n" @@ -"Generated-By: Babel 2.17.0\n" +"Generated-By: Babel 2.17.0\n"Also applies to: 10-12, 21-21
♻️ Duplicate comments (2)
docs/scripts/un_sdg/sdg7/CMakeLists.txt (1)
22-36: Verify that implicit dependencies track SQL file changes.The explicit
DEPENDS all_exercises_sdg7.sqlwas removed. As with the sdg3 script, ensure that changes toall_exercises_sdg7.sqlwill still trigger a rebuild, since the file is configured viaconfigure_file(line 19) but may not be automatically tracked without the explicitDEPENDSclause.docs/scripts/un_sdg/sdg11/CMakeLists.txt (1)
38-54: Verify that implicit dependencies track SQL file changes.The explicit
DEPENDS all_exercises_sdg11.sqlwas removed, consistent with the pattern in sdg3 and sdg7. Confirm that CMake will still detect changes toall_exercises_sdg11.sqland trigger rebuilds appropriately, despite the removal of the explicit dependency declaration.
🧹 Nitpick comments (3)
locale/en/LC_MESSAGES/basic/graphs.po (3)
33-35: Fix grammar: plural and phrasing.-"Different application require different graphs. This chapter covers how to " -"discard disconnected segments and different approaches to create graphs." +"Different applications require different graphs. This chapter covers how to " +"discard disconnected segments and approaches to create graphs."
41-47: Fix grammar: “consists of” and commas.-"In this chapter there are three graph requirements. It consists on three " -"graphs based on a **fully connected** graph derived from ``ways``: two for " -"different types of vehicles and one for pedestrian, the source and the " -"target in all of them are based on the ``source_osm`` and ``target_osm``." +"In this chapter there are three graph requirements. It consists of three " +"graphs based on a **fully connected** graph derived from ``ways``: two for " +"different types of vehicles and one for pedestrians. The ``source`` and " +"``target`` in all of them are based on ``source_osm`` and ``target_osm``."
102-105: Standardize units: use m/s instead of “mts/sec”.-"The speed is ``2 mts/sec``." +"The speed is ``2 m/s``." @@ -"The ``cost`` and ``reverse_cost`` are in terms of seconds with speed of ``2 " -"mts/sec``. (line **7**)" +"The ``cost`` and ``reverse_cost`` are in terms of seconds with speed of ``2 " +"m/s``. (line **7**)"Also applies to: 504-513
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.github/workflows/locale.yml(1 hunks)CMakeLists.txt(2 hunks)docs/scripts/basic/data/CMakeLists.txt(0 hunks)docs/scripts/basic/graphs/CMakeLists.txt(0 hunks)docs/scripts/basic/pedestrian/CMakeLists.txt(1 hunks)docs/scripts/basic/plpgsql_function/CMakeLists.txt(0 hunks)docs/scripts/basic/sql_function/CMakeLists.txt(0 hunks)docs/scripts/basic/vehicles/CMakeLists.txt(1 hunks)docs/scripts/un_sdg/sdg11/CMakeLists.txt(1 hunks)docs/scripts/un_sdg/sdg3/CMakeLists.txt(1 hunks)docs/scripts/un_sdg/sdg7/CMakeLists.txt(1 hunks)locale/en/LC_MESSAGES/basic/data.po(10 hunks)locale/en/LC_MESSAGES/basic/graphs.po(25 hunks)locale/en/LC_MESSAGES/basic/pedestrian.po(18 hunks)locale/en/LC_MESSAGES/basic/plpgsql_function.po(18 hunks)locale/en/LC_MESSAGES/general-intro/introduction.po(8 hunks)locale/pot/basic/data.pot(5 hunks)locale/pot/basic/graphs.pot(6 hunks)locale/pot/basic/pedestrian.pot(3 hunks)locale/pot/basic/plpgsql_function.pot(5 hunks)locale/pot/general-intro/introduction.pot(3 hunks)
💤 Files with no reviewable changes (4)
- docs/scripts/basic/data/CMakeLists.txt
- docs/scripts/basic/plpgsql_function/CMakeLists.txt
- docs/scripts/basic/graphs/CMakeLists.txt
- docs/scripts/basic/sql_function/CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (32)
.github/workflows/locale.yml (1)
54-61: LGTM! Necessary addition for locale processing.Adding
gettextto the dependencies is appropriate for the locale workflow, as it provides essential tools (msgfmt, xgettext, msgmerge) required for processing translation files during the Auckland localization update.docs/scripts/basic/vehicles/CMakeLists.txt (1)
30-33: LGTM! Formatting improved for consistency.The indentation adjustments improve code consistency within the
add_custom_commandblock. No functional changes.locale/pot/general-intro/introduction.pot (3)
2-17: LGTM! Metadata updated correctly for Auckland workshop.The POT file metadata has been properly updated:
- Copyright year extended to 2025
- License package name changed to Auckland
- Project version bumped to 3.1
- POT-Creation-Date set to the PR date
28-36: LGTM! Location references updated to Auckland.The workshop location and routing data references have been correctly updated from Belém to Auckland throughout the introduction content.
111-111: LGTM! Developer attribution updated for Auckland workshop.The developer/presenter attribution line correctly references the FOSS4G Auckland workshop.
locale/pot/basic/graphs.pot (3)
2-17: LGTM! Metadata updated consistently for Auckland.The POT file metadata aligns with the workshop update to Auckland 3.1, including the copyright year, creation date, and package name.
48-81: LGTM! Geographic references updated to Auckland area.Location references and descriptions have been properly updated from Belém to Auckland. The bounding box coordinates
(174.775,-36.855,174.76,-36.84)are valid for Auckland, New Zealand.
406-426: LGTM! Taxi bounding box coordinates consistent.The taxi circulation area bounding box correctly references the same Auckland coordinates as defined earlier in the file.
locale/pot/basic/plpgsql_function.pot (3)
2-17: LGTM! Metadata updated for Auckland workshop.The POT metadata has been correctly updated with the 2025 copyright, Auckland 3.1 version, and current POT-Creation-Date, consistent with other locale files in this PR.
216-220: LGTM! Test coordinates updated to Auckland locations.The example coordinates have been properly updated from Belém to Auckland:
- Departure:
(-36.8471438, 174.7710042)- Destination:
(-36.8524328, 174.766749)These coordinates represent valid locations in Auckland, New Zealand.
316-652: LGTM! Coordinates used consistently throughout examples.The Auckland test coordinates are correctly referenced in all example sections, maintaining consistency across the tutorial exercises.
locale/en/LC_MESSAGES/general-intro/introduction.po (2)
12-21: LGTM! PO file metadata refreshed appropriately.The POT-Creation-Date and Generated-By fields have been updated as expected when refreshing translations from the updated POT template. The preserved Belém references in the header (lines 2-10) are standard PO file behavior—translation file headers retain original context while the msgid fields update from the template.
34-126: LGTM! Message IDs updated to Auckland context.The msgid fields correctly reflect the Auckland workshop content from the updated POT template, including location references and workshop details.
docs/scripts/basic/pedestrian/CMakeLists.txt (1)
18-31: Verify missing DEPENDS for SQL scripts doesn’t break incremental builds
Custom commands invoking SQL files (e.g., pedestrian.sql, images.sql, sql_function.sql, all_exercises_sdg7.sql, all_exercises_sdg11.sql) lack DEPENDS clauses, so CMake won’t detect changes to those files—only the data target still depends on data.txt. Confirm if omitting DEPENDS is intentional.locale/en/LC_MESSAGES/basic/plpgsql_function.po (3)
12-21: LGTM: Metadata updates are consistent.The POT-Creation-Date and Generated-By fields have been updated appropriately to reflect the current build timestamp and Babel 2.17.0.
229-234: LGTM: Coordinates updated to Auckland context.The latitude/longitude test coordinates have been correctly updated to Auckland locations, consistent with the PR objective to switch from Belém to Auckland data.
683-687: LGTM: Example coordinates consistent with Auckland context.The departure and destination coordinates in the usage examples align with the Auckland data context established throughout the PR.
locale/pot/basic/pedestrian.pot (3)
2-11: LGTM: Project metadata updated to Auckland 3.1.The copyright year, license reference, Project-Id-Version, and POT-Creation-Date have been appropriately updated to reflect the Auckland workshop context.
295-308: LGTM: Example data updated with Auckland vertex identifiers.The note_1.txt content now references Auckland-specific vertex IDs and updated travel times, consistent with the new dataset.
389-389: LGTM: Location names updated to Auckland venues.The narrative now references "The Band Rotunda" and "Auckland University of Technology," which are appropriate Auckland landmarks.
locale/en/LC_MESSAGES/basic/data.po (3)
12-21: LGTM: Metadata updates are consistent.POT-Creation-Date and Generated-By fields updated appropriately.
90-93: LGTM: Workshop data context updated to Auckland.The narrative correctly specifies Auckland city data with a Jun 2025 snapshot, aligned with the PR objectives.
159-159: LGTM: File path updated to Auckland data.The osm2pgrouting data path now references
auckland_NZ.osm, consistent with the Auckland context.locale/pot/basic/data.pot (3)
2-11: LGTM: POT file metadata updated to Auckland 3.1.Copyright year, license, Project-Id-Version, and POT-Creation-Date appropriately reflect the Auckland workshop context.
76-76: LGTM: Data snapshot updated to Auckland Jun 2025.The workshop data description now correctly references Auckland city data with a Jun 2025 snapshot.
132-132: LGTM: File path consistent with Auckland context.The data file path
~/Desktop/workshop/auckland_NZ.osmaligns with the Auckland workshop data.locale/en/LC_MESSAGES/basic/pedestrian.po (3)
12-21: LGTM: Metadata updates are consistent.POT-Creation-Date and Generated-By fields updated appropriately.
345-358: LGTM: Example routing data updated to Auckland context.The note_1.txt content now uses Auckland-specific vertex identifiers (10840, 936, 10928, 12777) and corresponding travel times that align with the new dataset.
458-460: LGTM: Location narrative updated to Auckland venues.The interpretation text correctly references "The Band Rotunda" and "Auckland University of Technology," which are appropriate Auckland landmarks for the workshop context.
CMakeLists.txt (3)
19-27: Version bump looks consistent.City set to Auckland; 3.1.0-dev strings are coherent.
58-60: Download dir and data date align with 2025 Auckland.Looks good.
83-87: Points for examples look plausible for central Auckland.No further action.
Changes proposed in this pull request:
@pgRouting/admins
Summary by CodeRabbit
New Features
Documentation
Chores