Skip to content

Conversation

cvvergara
Copy link
Member

@cvvergara cvvergara commented Sep 30, 2025

Changes proposed in this pull request:

  • rename directories and files
    • On directories on docs/basic/scripts and docs/basic/images/
      • 'chapter_X` -> corresponding rst file name
      • Example: docs/basic/images/chapter8 -> docs/basic/images/plpgsql_function
  • Adjust build, locale and docs to new names
  • CI: refining locale build

@pgRouting/admins

Summary by CodeRabbit

  • New Features

    • Introduced per-format documentation build targets and a dedicated locale generation target (when available).
    • Added “Graphs” workshop content with updated scripts and outputs.
  • Documentation

    • Renamed “Graph Views” to “Graphs”; refreshed images, paths, and cross-references across Basic and UN SDG sections.
    • Reorganized Basic chapters (data, pedestrian, vehicles, graphs, SQL/plpgsql functions).
    • Removed legacy Makefile and LaTeX manual page.
  • Localization

    • Updated translation sources and POT/PO files (en, es, sv) to new document paths.
    • Improved PO maintenance during locale updates.
  • Chores

    • Simplified CI locale workflow.
    • Removed Transifex configuration.

- including locale on cycle
- cleanup update_locale.sh
@cvvergara cvvergara requested a review from iosefa September 30, 2025 04:14
Copy link

coderabbitai bot commented Sep 30, 2025

Walkthrough

Restructures docs build and localization: replaces Makefile-driven Sphinx with CMake per-format targets and conditional sphinx-intl locale generation; revises CI/workflow and locale update script; removes Transifex config; reorganizes basic docs, images, and scripts into new directories; adds graphs SQL and targets; updates PO/POT references accordingly; deletes unused files.

Changes

Cohort / File(s) Summary
CI/Locale tooling
.github/scripts/update_locale.sh, .github/workflows/locale.yml, .tx/config
Script: switch to -DBUILD_HTML=OFF -DBUILD_LOCALE=ON, guard PO processing on non-empty changes file, iterate adds via list, run msgattrib over PO files. Workflow: remove CMake configure step. Transifex config removed.
Docs build system
docs/CMakeLists.txt, docs/Makefile
Introduces per-format targets (${format}, ${format}-${lang}) and a locale target when sphinx-intl is available; warns and skips locale if absent. Removes Sphinx Makefile.
Docs structure and paths
docs/index.rst, docs/basic/CMakeLists.txt, docs/basic/data.rst, docs/basic/graphs.rst, docs/basic/pedestrian.rst, docs/basic/plpgsql_function.rst, docs/basic/sql_function.rst, docs/basic/vehicle.rst, docs/interactions/chapter-9.rst, docs/un_sdg/*
Replace graph_views with graphs; update image and script include paths to new directories (data, pedestrian, vehicles, graphs, sql_function, plpgsql_function); adjust toctree entry.
Images and copying rules
docs/basic/images/CMakeLists.txt, docs/basic/images/graphs/CMakeLists.txt, docs/basic/images/sql_function/CMakeLists.txt, docs/images/_dsZWQsq
Update image subdir list; add graphs image copy loop; remove several sql_function images; delete stray RSS/XML file.
Scripts build orchestration
docs/scripts/basic/CMakeLists.txt, docs/scripts/basic/data/CMakeLists.txt, docs/scripts/basic/pedestrian/CMakeLists.txt, docs/scripts/basic/sql_function/CMakeLists.txt, docs/scripts/basic/plpgsql_function/CMakeLists.txt, docs/scripts/basic/vehicles/CMakeLists.txt
Rename targets and inputs to new directories/files; adjust dependencies ordering among new targets; update commands and byproducts accordingly.
Graphs SQL feature
docs/scripts/basic/graphs/CMakeLists.txt, docs/scripts/basic/graphs/graphs.sql
Add basic_graphs_scripts target running graphs.sql. SQL creates/refreshes graph artifacts: vehicle_net (VIEW), taxi_net (VIEW), walk_net (MATERIALIZED VIEW), with component extraction and test queries.
SQL function script reduction
docs/scripts/basic/sql_function/sql_function.sql
Truncated to minimal placeholder (removes prior workflow steps).
Misc content adjustment
docs/scripts/basic/chapter_5/map_5.1.json, docs/source/_themes/bootstrap, docs/source/latex.rst
Remove two line features from JSON; delete theme path reference; remove LaTeX page.
Localization updates (EN/ES/SV)
locale/en/*, locale/es/*, locale/sv/*, locale/pot/*
Update references from graph_views to graphs; update paths for moved notes/images/scripts; regenerate POT/PO metadata and locations.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant CI as GitHub Workflow
  participant CMake as CMake (docs)
  participant Sphinx as Sphinx
  participant Intl as sphinx-intl

  Dev->>CI: Push / PR
  CI->>CMake: Configure with -DBUILD_LOCALE=ON/-DBUILD_HTML=OFF
  alt sphinx-intl found
    CMake->>Intl: sphinx-intl update (generate POT/PO)
    CMake->>Sphinx: Build per format (${format})
    CMake->>Sphinx: Build per lang (${format}-${lang})
  else sphinx-intl missing
    CMake-->>CI: Warn and skip locale generation
    CMake->>Sphinx: Build per format only
  end
Loading
sequenceDiagram
  autonumber
  actor Maint as Maintainer
  participant Script as update_locale.sh
  participant Git as git
  participant PO as *.po files

  Maint->>Script: Run update_locale.sh
  Script->>Script: Configure CMake (-DBUILD_LOCALE=ON)
  Script->>Script: Check locale_changes_po.txt exists and non-empty
  alt changes present
    Script->>PO: perl transforms on listed files
    Script->>PO: find locale/*.po | msgattrib --no-obsolete
    Script->>Git: Loop add POTs from locale_changes_po_pot.txt
  else no changes
    Script-->>Maint: Skip transforms/adds
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

administrative

Suggested reviewers

  • iosefa
  • sanak

Poem

Hop, hop! I shuffle docs with care,
New graphs bloom, locales in the air.
Make gives way to CMake flow,
Sphinx will build where formats go.
Nets for taxis, walks, and wheels—
Thump goes my foot: approved squeals! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The current title “Renaming directories files” is generic and unclear, lacking conjunctions and context to convey the primary change. It partially refers to the renaming but does not specify which directories or files or the scope of updates, making it difficult for reviewers to grasp the main intent at a glance. Please revise the title to clearly reflect the key changes, for example “Rename chapter-based docs directories and update build and locale references” to provide precise context.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
docs/scripts/basic/plpgsql_function/CMakeLists.txt (1)

16-42: Sync BYPRODUCTS entries to actual SQL outputs and correct typo
BYPRODUCTS still lists chapter-based names (excercise-8_*) but plpgsql_function.sql emits:

  • created_points.txt
  • views_vertices1.txt, views_vertices2.txt, views_vertices3.txt
  • exercise_8_3_1.txt … exercise_8_7_3.txt

Update the list to exactly match these filenames (use underscores) and rename “excercise” → “exercise”.

docs/scripts/basic/pedestrian/CMakeLists.txt (1)

22-27: Update BYPRODUCTS to match pedestrian.sql outputs

CMakeLists.txt (docs/scripts/basic/pedestrian/CMakeLists.txt — lines 22–27) currently lists exercise_5_1.txt–exercise_5_6.txt but pedestrian.sql emits:
exercise_5_0.txt
exercise_5_1.txt
exercise_5_2.txt
exercise_5_3.txt
exercise_5_4.txt
exercise_5_5.txt
exercise_5_6.txt
exercise_5_7.txt
note_1.txt
note_2.txt

Update the BYPRODUCTS block to include the full set above.

docs/scripts/basic/sql_function/CMakeLists.txt (1)

13-32: Rename BYPRODUCTS output files and update all their references to match the new sql_function naming

  • Update the BYPRODUCTS entries in docs/scripts/basic/sql_function/CMakeLists.txt
  • Update the \o exercise_7_*.txt lines in docs/scripts/basic/sql_function/sql_function.sql
  • Update the :start-after, :end-before and literalinclude entries in docs/basic/sql_function.rst to reference the new filenames instead of exercise_7_*.txt
🧹 Nitpick comments (6)
.github/scripts/update_locale.sh (1)

47-47: Consider adding error handling for missing files.

The loop correctly stages all changed locale files. However, if any file listed in locale_changes_po_pot.txt doesn't exist, git add will emit an error but the script will continue. Since the script doesn't use set -e, this is consistent with the existing error-handling approach, but consider whether failures should be detected.

If stricter error handling is desired, consider:

-while read -r f; do git add "$f"; done < build/docs/locale_changes_po_pot.txt
+while read -r f; do 
+  if ! git add "$f"; then
+    echo "Warning: Failed to add $f" >&2
+  fi
+done < build/docs/locale_changes_po_pot.txt
docs/scripts/basic/vehicles/CMakeLists.txt (1)

16-28: Consider renaming BYPRODUCTS to align with the new structure.

The BYPRODUCTS still use the old naming convention (section-6.1-1.txt, section-6.1.1.txt, etc.) while the target and primary files have been renamed to vehicles. For consistency, consider whether these output files should also follow the new semantic naming (e.g., vehicles-*.txt).

If the current naming is intentional (e.g., to maintain compatibility with documentation references), this is acceptable, but it creates a small inconsistency.

docs/scripts/basic/graphs/graphs.sql (2)

67-82: Consider extracting the max component CTE to reduce duplication.

The CTE at lines 69-74 duplicates the same logic from lines 59-62. If this script is frequently modified, consider creating a temporary view or function to identify the_component and reuse it across vehicle_net, taxi_net, and walk_net definitions.

For example, create a reusable view before defining the network views:

CREATE OR REPLACE VIEW max_component_view AS
WITH
all_components AS (SELECT component, count(*) FROM ways GROUP BY component),
max_component AS (SELECT max(count) from all_components)
SELECT component FROM all_components WHERE count = (SELECT max FROM max_component);

Then reference it in your network views:

CREATE VIEW vehicle_net AS
SELECT
  gid AS id,
  source_osm AS source, target_osm AS target,
  cost_s AS cost, reverse_cost_s AS reverse_cost,
  name, length_m AS length, the_geom AS geom
FROM ways 
JOIN max_component_view USING (component) 
JOIN configuration USING (tag_id)
WHERE tag_value NOT IN ('steps','footway','path','cycleway');

110-123: Consider consistency in CTE naming and verify materialized view refresh strategy.

  1. The component-finding CTE (lines 112-115) is duplicated for a third time with different variable names (allc, maxcount vs all_components, max_component). Consider the refactoring suggestion from the vehicle_net comment.

  2. Since walk_net is a MATERIALIZED VIEW, ensure there's a strategy to refresh it when the underlying ways table changes, otherwise it may become stale.

Add a comment documenting when/how to refresh:

-- MATERIALIZED VIEW walk_net must be refreshed when ways data changes:
-- REFRESH MATERIALIZED VIEW walk_net;
CREATE MATERIALIZED VIEW walk_net AS
...
docs/CMakeLists.txt (2)

58-73: Clarify target name in add_dependencies for better readability.

On line 72, add_dependencies("${format}" ...) uses the loop variable ${format}, which is "locale" in this branch. While functionally correct, it's less clear than using the literal string "locale" or the previously defined target name. Consider:

-      add_dependencies("${format}" examples general-intro advanced  appendix  basic un_sdg scripts interactions)
+      add_dependencies(locale examples general-intro advanced  appendix  basic un_sdg scripts interactions)

This improves readability and makes the dependency relationship explicit.


93-109: Initialize sphinx_flags and isolate doctrees per language

  • sphinx_flags isn’t set anywhere in the CMake files—in docs/CMakeLists.txt it expands to an empty string by default. Add an explicit set(sphinx_flags "" CACHE STRING "Optional flags for sphinx-build") (in the root or docs/CMakeLists.txt) to avoid confusion.
  • Using a shared doctrees directory (–d ${CMAKE_CURRENT_BINARY_DIR}/doctrees) can collide when building formats/languages in parallel. Scope it per build, e.g.:
    -d ${CMAKE_CURRENT_BINARY_DIR}/doctrees/${format}/${lang}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6371501 and 5e32488.

⛔ Files ignored due to path filters (30)
  • docs/basic/images/data/prepareData.png is excluded by !**/*.png
  • docs/basic/images/graphs/ch7-e3.png is excluded by !**/*.png
  • docs/basic/images/graphs/taxi_net.png is excluded by !**/*.png
  • docs/basic/images/graphs/vehicle_net.png is excluded by !**/*.png
  • docs/basic/images/graphs/walk_net.png is excluded by !**/*.png
  • docs/basic/images/pedestrian/pedestrian_combinations.png is excluded by !**/*.png
  • docs/basic/images/pedestrian/pedestrian_route1.png is excluded by !**/*.png
  • docs/basic/images/pedestrian/pedestrian_route2.png is excluded by !**/*.png
  • docs/basic/images/pedestrian/pedestrian_route4.png is excluded by !**/*.png
  • docs/basic/images/pedestrian/pedestrian_route5.png is excluded by !**/*.png
  • docs/basic/images/pedestrian/route.png is excluded by !**/*.png
  • docs/basic/images/plpgsql_function/ch8-e7.png is excluded by !**/*.png
  • docs/basic/images/plpgsql_function/ch8-taxinet.png is excluded by !**/*.png
  • docs/basic/images/plpgsql_function/ch8-vehiclenet.png is excluded by !**/*.png
  • docs/basic/images/plpgsql_function/ch8-walknet.png is excluded by !**/*.png
  • docs/basic/images/plpgsql_function/closest_vertex.png is excluded by !**/*.png
  • docs/basic/images/sql_function/ch7-e4.png is excluded by !**/*.png
  • docs/basic/images/sql_function/ch7-e5.png is excluded by !**/*.png
  • docs/basic/images/sql_function/ch7-e6.png is excluded by !**/*.png
  • docs/basic/images/sql_function/ch7-e7.png is excluded by !**/*.png
  • docs/basic/images/sql_function/ch7-e8-1.png is excluded by !**/*.png
  • docs/basic/images/sql_function/ch7-e8.png is excluded by !**/*.png
  • docs/basic/images/vehicle/ad11.png is excluded by !**/*.png
  • docs/basic/images/vehicle/ad7.png is excluded by !**/*.png
  • docs/basic/images/vehicle/ad8.png is excluded by !**/*.png
  • docs/basic/images/vehicle/pedestrian_only_roads.png is excluded by !**/*.png
  • docs/basic/images/vehicle/route_using_pedestrian.png is excluded by !**/*.png
  • docs/images/ch7-e1.png is excluded by !**/*.png
  • docs/images/chapter7/ch7-e2-1.png is excluded by !**/*.png
  • docs/scripts/basic/chapter_5/example.svg is excluded by !**/*.svg
📒 Files selected for processing (58)
  • .github/scripts/update_locale.sh (1 hunks)
  • .github/workflows/locale.yml (0 hunks)
  • .tx/config (0 hunks)
  • docs/CMakeLists.txt (1 hunks)
  • docs/Makefile (0 hunks)
  • docs/basic/CMakeLists.txt (1 hunks)
  • docs/basic/data.rst (2 hunks)
  • docs/basic/graphs.rst (11 hunks)
  • docs/basic/images/CMakeLists.txt (1 hunks)
  • docs/basic/images/graphs/CMakeLists.txt (1 hunks)
  • docs/basic/images/sql_function/CMakeLists.txt (0 hunks)
  • docs/basic/pedestrian.rst (16 hunks)
  • docs/basic/plpgsql_function.rst (13 hunks)
  • docs/basic/sql_function.rst (17 hunks)
  • docs/basic/vehicle.rst (18 hunks)
  • docs/images/_dsZWQsq (0 hunks)
  • docs/index.rst (1 hunks)
  • docs/interactions/chapter-9.rst (1 hunks)
  • docs/scripts/basic/CMakeLists.txt (2 hunks)
  • docs/scripts/basic/chapter_5/map_5.1.json (0 hunks)
  • docs/scripts/basic/data/CMakeLists.txt (2 hunks)
  • docs/scripts/basic/graphs/CMakeLists.txt (1 hunks)
  • docs/scripts/basic/graphs/graphs.sql (1 hunks)
  • docs/scripts/basic/pedestrian/CMakeLists.txt (3 hunks)
  • docs/scripts/basic/plpgsql_function/CMakeLists.txt (2 hunks)
  • docs/scripts/basic/sql_function/CMakeLists.txt (2 hunks)
  • docs/scripts/basic/sql_function/sql_function.sql (0 hunks)
  • docs/scripts/basic/vehicles/CMakeLists.txt (2 hunks)
  • docs/source/_themes/bootstrap (0 hunks)
  • docs/source/latex.rst (0 hunks)
  • docs/un_sdg/data.rst (1 hunks)
  • docs/un_sdg/sdg11-cities.rst (1 hunks)
  • docs/un_sdg/sdg3-health.rst (1 hunks)
  • docs/un_sdg/sdg7-energy.rst (1 hunks)
  • locale/en/LC_MESSAGES/basic/graphs.po (2 hunks)
  • locale/en/LC_MESSAGES/basic/pedestrian.po (2 hunks)
  • locale/en/LC_MESSAGES/basic/plpgsql_function.po (2 hunks)
  • locale/en/LC_MESSAGES/un_sdg/sdg11-cities.po (1 hunks)
  • locale/en/LC_MESSAGES/un_sdg/sdg3-health.po (1 hunks)
  • locale/en/LC_MESSAGES/un_sdg/sdg7-energy.po (1 hunks)
  • locale/es/LC_MESSAGES/basic/graphs.po (7 hunks)
  • locale/es/LC_MESSAGES/basic/pedestrian.po (2 hunks)
  • locale/es/LC_MESSAGES/basic/plpgsql_function.po (2 hunks)
  • locale/es/LC_MESSAGES/un_sdg/sdg11-cities.po (1 hunks)
  • locale/es/LC_MESSAGES/un_sdg/sdg3-health.po (1 hunks)
  • locale/es/LC_MESSAGES/un_sdg/sdg7-energy.po (1 hunks)
  • locale/pot/basic/graphs.pot (1 hunks)
  • locale/pot/basic/pedestrian.pot (2 hunks)
  • locale/pot/basic/plpgsql_function.pot (2 hunks)
  • locale/pot/un_sdg/sdg11-cities.pot (1 hunks)
  • locale/pot/un_sdg/sdg3-health.pot (1 hunks)
  • locale/pot/un_sdg/sdg7-energy.pot (1 hunks)
  • locale/sv/LC_MESSAGES/basic/graphs.po (1 hunks)
  • locale/sv/LC_MESSAGES/basic/pedestrian.po (2 hunks)
  • locale/sv/LC_MESSAGES/basic/plpgsql_function.po (2 hunks)
  • locale/sv/LC_MESSAGES/un_sdg/sdg11-cities.po (1 hunks)
  • locale/sv/LC_MESSAGES/un_sdg/sdg3-health.po (1 hunks)
  • locale/sv/LC_MESSAGES/un_sdg/sdg7-energy.po (1 hunks)
💤 Files with no reviewable changes (9)
  • docs/basic/images/sql_function/CMakeLists.txt
  • docs/scripts/basic/chapter_5/map_5.1.json
  • docs/images/_dsZWQsq
  • .tx/config
  • docs/Makefile
  • docs/source/_themes/bootstrap
  • docs/source/latex.rst
  • .github/workflows/locale.yml
  • docs/scripts/basic/sql_function/sql_function.sql
⏰ 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 (64)
.github/scripts/update_locale.sh (3)

26-26: LGTM!

The CMake configuration appropriately enables locale generation while disabling HTML build for a focused locale update.


43-45: LGTM!

The find command correctly removes obsolete entries from all .po files using msgattrib. The sh -c invocation properly passes the filename as $1, and the quoting is safe.


35-40: Approve English‐only filter logic. The en/ filter matches the only entries present in locale_changes_po.txt (Sphinx-intl is invoked with -l en), so no changes are needed.

docs/un_sdg/data.rst (1)

13-13: LGTM! Image path updated consistently.

The image path has been correctly updated from chapter4 to data, aligning with the semantic directory restructuring described in the PR objectives.

locale/sv/LC_MESSAGES/un_sdg/sdg7-energy.po (1)

137-140: LGTM! Documentation cross-reference updated consistently.

The documentation reference has been correctly updated from graph_views to graphs in both the source string (msgid) and Swedish translation (msgstr), maintaining consistency with the broader documentation restructuring.

docs/interactions/chapter-9.rst (1)

14-14: LGTM! Image path updated to semantic naming.

The image path has been correctly updated from chapter5 to pedestrian, consistent with the directory restructuring to use more meaningful names instead of chapter numbers.

locale/pot/un_sdg/sdg7-energy.pot (1)

108-108: LGTM! Translation template updated consistently.

The documentation cross-reference in the translation template has been correctly updated from graph_views to graphs, ensuring all language translations will inherit this change.

locale/pot/un_sdg/sdg11-cities.pot (1)

205-205: LGTM! Translation template cross-reference updated.

The documentation reference has been correctly updated from graph_views to graphs in the translation template, maintaining consistency with the project-wide documentation restructuring.

docs/basic/CMakeLists.txt (1)

12-12: LGTM! Consistent documentation reference update.

The update from graph_views.rst to graphs.rst aligns with the PR's systematic refactoring from chapter-based to semantic naming conventions. This change is consistent with corresponding updates in locale files and documentation cross-references throughout the repository.

locale/en/LC_MESSAGES/un_sdg/sdg7-energy.po (2)

142-143: LGTM! Documentation cross-reference correctly updated.

The cross-reference update from :doc:../basic/graph_views to `:doc:`../basic/graphs is consistent with the PR-wide documentation restructuring and matches parallel updates in Spanish, Swedish, and POT locale files.


189-189: LGTM! Second cross-reference correctly updated.

The msgstr translation correctly mirrors the source string's documentation reference change to graphs, maintaining consistency with the updated English documentation structure.

locale/es/LC_MESSAGES/un_sdg/sdg7-energy.po (1)

184-189: LGTM! Spanish locale correctly updated.

Both the source message (line 184) and Spanish translation (line 189) correctly reference :doc:../basic/graphs`` instead of the old graph_views path. This maintains consistency across English and Spanish documentation.

docs/un_sdg/sdg7-energy.rst (1)

100-100: LGTM! Source documentation reference correctly updated.

The documentation cross-reference update from graph_views to graphs is the source change that correctly propagates to all locale translations. This aligns with the PR's systematic directory and file renaming strategy.

locale/es/LC_MESSAGES/basic/pedestrian.po (2)

395-413: LGTM! Script path references correctly updated.

The references to note_1.txt have been updated from generic paths to the pedestrian-specific subdirectory (pedestrian/note_1.txt). This change is consistent with the PR's restructuring of documentation assets from chapter-based to content-based organization.


525-533: LGTM! Second script path reference correctly updated.

The reference to note_2.txt correctly points to the new pedestrian/note_2.txt path, maintaining consistency with the broader directory reorganization in this PR.

docs/basic/images/graphs/CMakeLists.txt (2)

13-18: LGTM!

The file copy logic is correct and follows the standard CMake pattern. The optional debug logging is a nice touch for troubleshooting.


5-10: Image file existence verified. All four images referenced in docs/basic/images/graphs/CMakeLists.txt are present in docs/basic/images/graphs/, so no further action is required.

locale/en/LC_MESSAGES/basic/pedestrian.po (1)

339-357: LGTM!

The translation source references have been correctly updated to reflect the directory renaming from chapter_5 to pedestrian. The message strings remain unchanged, which is appropriate for this refactoring.

Also applies to: 455-460

docs/basic/pedestrian.rst (1)

13-363: LGTM!

All resource paths have been systematically updated to reflect the directory reorganization from chapter_5 to pedestrian. The changes maintain proper relative path structure and are consistent throughout the file.

docs/basic/images/CMakeLists.txt (2)

18-23: LGTM!

The foreach loop correctly iterates over the updated subdirectory list and copies each directory to the build location. The logic remains sound after the directory renaming.


5-12: All new image subdirectories verified: each of the six renamed directories exists with files as expected. No further changes needed.

docs/basic/vehicle.rst (1)

14-370: Directory naming is correct: both docs/basic/images/vehicle and docs/scripts/basic/vehicles exist as referenced; no changes needed.

locale/sv/LC_MESSAGES/basic/pedestrian.po (2)

338-357: LGTM! Path references updated correctly.

The source file path references have been updated from chapter_5 to pedestrian to align with the directory restructuring. The translation strings remain unchanged, which is appropriate.


452-457: LGTM! Path references updated correctly.

The source file path references for note_2.txt have been updated from chapter_5 to pedestrian, consistent with the broader renaming effort.

locale/sv/LC_MESSAGES/un_sdg/sdg11-cities.po (1)

254-257: LGTM! Documentation reference updated correctly.

The cross-reference has been updated from graph_views to graphs to align with the documentation restructuring. This change is consistent with the broader renaming effort across the project.

locale/en/LC_MESSAGES/un_sdg/sdg3-health.po (1)

480-482: LGTM! Documentation reference updated correctly.

The cross-reference has been updated from graph_views to graphs, aligning with the documentation restructuring across the project.

locale/en/LC_MESSAGES/un_sdg/sdg11-cities.po (1)

257-257: LGTM! Documentation reference updated correctly.

The cross-reference has been updated from graph_views to graphs, maintaining consistency with the documentation restructuring.

docs/un_sdg/sdg3-health.rst (1)

424-424: LGTM! Documentation cross-reference updated correctly.

The Sphinx cross-reference has been updated from graph_views to graphs, aligning with the documentation restructuring. This change is consistent with the corresponding updates in all translation files.

locale/en/LC_MESSAGES/basic/graphs.po (1)

23-663: LGTM! Consistent reference updates from graph_views to graphs.

All changes systematically update documentation path references from graph_views.rst to graphs.rst in the PO file metadata. The actual translation strings remain unchanged, and the updates are consistent throughout the file.

docs/index.rst (1)

36-36: LGTM! Documentation reference correctly updated.

The toctree reference has been updated from basic/graph_views.rst to basic/graphs.rst, aligning with the file renaming changes across the repository.

docs/un_sdg/sdg11-cities.rst (1)

242-242: LGTM! Cross-reference updated correctly.

The Sphinx doc reference has been updated from ../basic/graph_views to ../basic/graphs, maintaining consistency with the documentation restructuring.

locale/pot/un_sdg/sdg3-health.pot (1)

391-392: LGTM! Translation template reference updated.

The doc reference in the msgid has been correctly updated from ../basic/graph_views to ../basic/graphs, ensuring translation templates reflect the new documentation structure.

locale/es/LC_MESSAGES/basic/plpgsql_function.po (1)

301-348: LGTM! Spanish translation references updated consistently.

All Spanish translation strings and their corresponding English source strings have been updated to reference :doc:graphs instead of `:doc:`graph_views, maintaining consistency with the documentation restructuring.

locale/es/LC_MESSAGES/un_sdg/sdg11-cities.po (1)

317-317: LGTM – Documentation reference updated correctly.

The cross-reference to the graphs documentation has been updated from graph_views to graphs, aligning with the directory/file renaming in this PR. The Spanish translation text remains unchanged.

locale/pot/basic/plpgsql_function.pot (1)

91-93: LGTM – POT template references updated consistently.

All three documentation cross-references have been correctly updated from graph_views to graphs:

  • Line 91: pgr_extractVertices guidance
  • Line 93: JOIN operation description
  • Line 122: See reference

The changes are consistent with the broader documentation refactor in this PR.

Also applies to: 122-122

locale/sv/LC_MESSAGES/un_sdg/sdg3-health.po (1)

473-476: LGTM – Swedish translation reference updated correctly.

Both the source msgid and Swedish translation msgstr have been updated to reference :doc:../basic/graphs`` instead of graph_views. The Swedish translation text itself remains unchanged, only the documentation target was updated.

locale/pot/basic/graphs.pot (1)

19-593: LGTM – POT template metadata updated for renamed source file.

This POT template file has been updated to reflect the renaming of the source documentation file from graph_views.rst to graphs.rst. All source location references (lines with #: comments) now correctly point to basic/graphs.rst instead of the old path. This is a necessary metadata update that maintains synchronization between the translation catalog and the renamed source documentation.

locale/en/LC_MESSAGES/basic/plpgsql_function.po (1)

285-285: LGTM – English translation references updated consistently.

All three documentation cross-references in the English translation have been correctly updated from graph_views to graphs:

  • Line 285: pgr_extractVertices explanation reference
  • Line 292: JOIN operation reference to ways_vertices
  • Line 322: See reference for disconnected components

The changes align with the documentation restructuring in this PR.

Also applies to: 292-292, 322-322

docs/basic/sql_function.rst (2)

85-86: LGTM! Path updates are consistent.

All literalinclude and image path updates correctly change from chapter_8 or generic paths to the new sql_function directory structure. The changes align with the PR's objective to use descriptive names instead of chapter numbers.

Also applies to: 109-114, 118-118, 132-134, 165-170, 176-177, 203-208, 212-213, 235-238, 240-241, 271-276, 280-281, 286-289, 291-291, 327-330, 334-335, 373-377, 386-391, 395-396, 414-417, 421-422, 425-428, 432-433, 436-438, 442-443


14-14: No changes needed for the image path.

The route.png file lives only under docs/basic/images/pedestrian/route.png, so the reference in sql_function.rst is correct.

locale/sv/LC_MESSAGES/basic/plpgsql_function.po (1)

284-287: LGTM! Documentation cross-references updated consistently.

The Swedish translation correctly updates all references from graph_views to graphs, aligning with the documentation restructuring. Both source strings (msgid) and translations (msgstr) are updated consistently.

Also applies to: 291-293, 323-324

locale/es/LC_MESSAGES/un_sdg/sdg3-health.po (1)

591-596: LGTM! Cross-reference updated consistently.

The Spanish translation correctly updates the documentation reference from graph_views to graphs in the Exercise 12 description, maintaining consistency with the broader documentation restructuring.

docs/basic/data.rst (1)

14-14: LGTM! Path updates are consistent.

All image and script references correctly updated from chapter_4 to data directory structure, aligning with the PR's goal to replace chapter numbers with descriptive names.

Also applies to: 150-156

locale/pot/basic/pedestrian.pot (1)

290-309: LGTM! Translation template paths updated consistently.

The POT file correctly updates all script references from chapter_5 to pedestrian directory structure for both note_1.txt and note_2.txt references, ensuring translation files will reference the correct paths.

Also applies to: 388-390

locale/es/LC_MESSAGES/basic/graphs.po (1)

15-15: LGTM! Consistent cross-reference updates.

The file systematically updates all references from graph_views.rst to graphs.rst, aligning with the directory and file renaming objectives of this PR. The Spanish translations remain unchanged, and only the source document references in msgid entries are updated.

Also applies to: 24-733

docs/scripts/basic/CMakeLists.txt (2)

6-13: LGTM! Clear semantic naming.

The directory list has been successfully refactored from numbered chapters (chapter_4 through chapter_8) to meaningful semantic names (data, pedestrian, vehicles, graphs, sql_function, plpgsql_function). This improves maintainability and makes the build structure more intuitive.


23-27: Confirm dual dependency basic_sql_function_scripts intentionally depends on both basic_vehicles_scripts and basic_graphs_scripts, and basic_graphs_scripts has no dependency on vehicles—no circular ordering issues.

docs/scripts/basic/vehicles/CMakeLists.txt (1)

1-1: LGTM! Consistent target and file renaming.

The target has been successfully renamed from basic_chapter_6_scripts to basic_vehicles_scripts, and the primary SQL file has been updated from all_exercises.sql to vehicles.sql. The comment and dependencies are appropriately updated.

Also applies to: 6-6, 14-14, 30-30, 32-33

docs/basic/plpgsql_function.rst (3)

13-13: LGTM! Documentation cross-references updated correctly.

The image path and cross-references to the graphs documentation have been properly updated from the old graph_views to the new graphs naming convention.

Also applies to: 91-91, 94-94, 122-122


98-98: LGTM! Script paths consistently updated.

All literalinclude directives have been systematically updated from ../scripts/basic/chapter_8/ to ../scripts/basic/plpgsql_function/, aligning with the directory restructuring.

Also applies to: 106-106, 116-116, 148-148, 161-161, 178-178, 186-186, 233-233, 282-282, 295-295, 349-349, 382-382, 391-391, 435-435, 448-448, 459-459


243-243: LGTM! Image paths migrated to plpgsql_function.

Image references have been correctly updated from images/chapter8/ to images/plpgsql_function/, consistent with the PR's renaming objectives.

Also applies to: 247-247, 251-251, 400-400

docs/scripts/basic/data/CMakeLists.txt (1)

5-5: LGTM! Comprehensive renaming from section-based to semantic naming.

The CMakeLists.txt has been completely and consistently updated:

  • Target renamed from basic_chapter_4_scripts to basic_data_scripts
  • Script file renamed from section-4.3.2.sh to data.sh
  • Output file renamed from section-4.3.2.txt to data.txt

All references are updated consistently throughout the file.

Also applies to: 16-18, 21-22

docs/basic/graphs.rst (1)

1-624: LGTM! Comprehensive and consistent path updates.

All image and script references have been systematically updated from chapter-based naming (e.g., chapter_7, chapter5) to feature-based naming (graphs, pedestrian). The literalinclude blocks, image paths, and documentation anchors are all consistently aligned with the new directory structure.

locale/sv/LC_MESSAGES/basic/graphs.po (1)

1-650: LGTM! Locale file correctly updated.

All documentation references have been consistently updated from graph_views.rst to graphs.rst throughout the Swedish translation file. The msgstr translations remain appropriately unchanged as the rename is structural, not content-related.

docs/scripts/basic/graphs/CMakeLists.txt (1)

1-50: LGTM! Well-structured new build target.

The new basic_graphs_scripts target follows a clean, feature-based naming convention with descriptive byproduct names (e.g., create_vertices.txt, fill_columns_1.txt, test_view1.txt). This is a significant improvement over chapter-based numbering and makes the build outputs more understandable.

docs/scripts/basic/graphs/graphs.sql (4)

2-4: LGTM! Safe cleanup approach.

The DROP statements correctly use IF EXISTS guards and CASCADE to handle dependent objects.


32-62: LGTM! Component analysis logic is sound.

The two-phase approach (assign components to vertices, then propagate to ways) is correct. The CTE logic properly identifies the largest connected component(s) and handles potential ties.


93-100: LGTM! Efficient taxi network derivation.

Deriving taxi_net from vehicle_net with a cost multiplier and spatial filter is appropriate. The 10% cost increase and bounding box restriction effectively model taxi-specific routing constraints.


8-29: Confirm vertex extraction and join logic. Ensure the ways table defines source_osm and target_osm as the same OSM node identifiers used by pgr_extractVertices, and that the function’s id output matches those node values—otherwise the UPDATE statements to fill geom will not match any rows.

docs/CMakeLists.txt (3)

46-49: LGTM! Clear separation of locale vs documentation format handling.

The loop structure appropriately distinguishes between the special locale target and regular documentation formats (HTML, PDF, etc.).


84-89: LGTM! Appropriate per-format target and landing page setup.

Creating a per-format target and configuring a forward page as index.html provides a clean entry point for multi-language documentation builds.


107-113: LGTM! Dependency chain properly wired.

The dependency hierarchy (sections → format-language → format → doc) ensures that documentation builds in the correct order and all artifacts are generated before higher-level targets complete.

docs/scripts/basic/sql_function/CMakeLists.txt (2)

6-6: LGTM! SQL file renamed consistently.

The rename from all_sections.sql to sql_function.sql is reflected consistently in the configure_file, psql command, and DEPENDS clause.


1-1: Approve rename; references consistent

No occurrences of basic_chapter_7_scripts remain and basic_sql_function_scripts is defined and referenced correctly.

@cvvergara cvvergara merged commit 9ab04b5 into pgRouting:develop Sep 30, 2025
2 checks passed
@cvvergara cvvergara deleted the renaming-directories-files branch September 30, 2025 11:34
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.

2 participants