Standardize ordering functions code #3026
Conversation
WalkthroughReplaces template/class-based ordering implementations with concrete free functions in pgrouting::functions, introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant SQL as SQL caller (SRF)
participant SRF as C wrapper (_pgr_*ordering)
participant Proc as pgr_process_ordering
participant Impl as pgrouting::functions (ordering impls)
participant ToPG as to_postgres::get_vertexId
SQL->>SRF: call SRF function (edges_sql)
SRF->>Proc: pgr_process_ordering(edges_sql, Which)
Proc->>Impl: build graph & call selected ordering
Impl-->>Proc: std::vector<vertex_descriptor>
Proc->>ToPG: get_vertexId(graph, vector, &count, &int64_t_array)
ToPG-->>Proc: int64_t* results, count
Proc-->>SRF: return_count, return_tuples (int64_t*)
SRF-->>SQL: produce SRF rows from return_tuples
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ordering/ordering_driver.cpp (1)
153-155: Logic error: Conditional appears inverted.The ternary condition
hint.empty()? to_pg_msg(hint) : to_pg_msg(log)seems backwards:
- When
hintis empty, it assigns the empty string tolog_msg- When
hinthas content, it ignoreshintand usesloginsteadThis should likely be
!hint.empty()to provide the hint when available.🔎 Proposed fix
} catch (const std::string &ex) { *err_msg = to_pg_msg(ex); - *log_msg = hint.empty()? to_pg_msg(hint) : to_pg_msg(log); + *log_msg = hint.empty()? to_pg_msg(log) : to_pg_msg(hint); }
🤖 Fix all issues with AI Agents
In @include/cpp_common/to_postgres.hpp:
- Around line 84-86: The function get_vertexId currently takes the parameter
data by value, causing an unnecessary copy; change its signature to take const
std::vector<typename G::V>& data (i.e., pass by const reference) and update any
internal usage accordingly to avoid modifying the vector; ensure any forward
declarations or overloads of get_vertexId use the same const-ref signature to
maintain consistency.
- Around line 93-95: The loop variable `e` in the for-loop over `data` is
misleading because it represents a vertex descriptor; rename it to a clearer
name such as `v` or `vertex` in the loop header and update all uses inside the
loop (e.g., change `graph.graph[e].id` to `graph.graph[v].id`) so the intent of
the code is obvious; ensure the change is applied to the for-loop that assigns
into `(*result_tuples)[seq]` and increments `seq` to avoid any breakage.
In @src/ordering/cuthillMckeeOrdering.c:
- Around line 55-59: Introduce named constants for ordering types (e.g., an enum
or #defines) in include/process/ordering_process.h (names like ORDERING_SLOAN,
ORDERING_CUTHILL_MCKEE, ORDERING_KING, ORDERING_TOPOLOGICAL) and update call
sites to use them instead of literals; specifically replace the hardcoded "1" in
the pgr_process_ordering invocation with the new ORDERING_CUTHILL_MCKEE constant
and update any comparisons in ordering_process.cpp to reference the new
constants so all ordering type checks and calls use the shared identifiers.
In @src/ordering/cuthillMckeeOrdering.cpp:
- Around line 30-40: The source file cuthillMckeeOrdering.cpp is missing its
corresponding header include; add #include "ordering/cuthillMckeeOrdering.hpp"
at the top of cuthillMckeeOrdering.cpp (alongside the other includes) so the
function/class declarations in ordering::cuthillMckeeOrdering remain
synchronized with their definitions.
In @src/ordering/ordering_driver.cpp:
- Around line 110-115: There are two issues: remove the stray double semicolon
after the DirectedGraph construction (extra ';' after DirectedGraph() in the
expression creating digraph) and verify/fix the vertex extraction predicate:
currently vertices is only populated when which == 0 or which == 2 via
pgrouting::extract_vertices(edges), so confirm whether Cuthill-McKee (which ==
1) and Topological Sort (which == 11) should also extract vertices; if they
should, update the condition to include those codes (e.g., include 1 and/or 11)
or otherwise document/leave as-is; locate symbols: the conditional initializing
vertices, the call pgrouting::extract_vertices(edges), and the constructors
UndirectedGraph(vertices) and DirectedGraph(vertices) to make the change.
- Around line 118-135: The switch on variable which in this function lacks a
default path; add a default case to detect unexpected which values and fail
fast: inside the default, log or throw a descriptive std::invalid_argument (or
runtime_error) that includes the invalid which value, and ensure you do not
silently leave return_count/return_tuples unchanged. Update the switch
containing cases for sloanOrdering, cuthillMckeeOrdering, kingOrdering and
topologicalSort to include this default so callers get an explicit error instead
of "No results found".
In @src/ordering/sloanOrdering.cpp:
- Around line 1-29: The file header comment incorrectly names the file as
"sloanOrdering.hpp"; update that header line to "sloanOrdering.cpp" so the
comment matches the actual source file (look for the top-of-file comment block
that currently contains "File: sloanOrdering.hpp" and replace it with "File:
sloanOrdering.cpp").
In @src/ordering/topologicalSort.c:
- Around line 55-59: Replace the magic number 11 passed to pgr_process_ordering
with a named constant to improve readability and maintainability: define a
shared constant (e.g., ORDERING_TOPOLOGICAL_SORT) in a common header used by
both C and C++ code (for example ordering_process.h or the header included by
ordering_driver.cpp), include that header where topologicalSort.c calls
pgr_process_ordering, and pass ORDERING_TOPOLOGICAL_SORT instead of the literal
11 in the pgr_process_ordering call.
In @src/ordering/topologicalSort.cpp:
- Around line 57-66: The catch block that currently does throw
std::string("Graph is not DAG") should throw a proper exception type instead;
replace the string throw in the catch(boost::exception const& ex) handler with a
std::runtime_error (or a project-specific exception) that includes diagnostic
info (e.g., boost::diagnostic_information(ex) or ex.what()) to preserve error
details and proper exception semantics; update any callers/tests if they expect
a string exception to handle the new exception type thrown by the
topological_sort error path.
- Line 51: Remove the dead local variable by deleting the declaration
"std::vector<int64_t> results;" from the topological sort function (the unused
variable named results in topologicalSort.cpp); ensure no other code references
that symbol and rebuild to confirm no compilation errors.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (23)
NEWS.mddoc/src/pgRouting-introduction.rstdoc/src/release_notes.rstinclude/c_types/i_rt.hinclude/cpp_common/to_postgres.hppinclude/drivers/ordering/cuthillMckeeOrdering_driver.hinclude/drivers/ordering/topologicalSort_driver.hinclude/ordering/cuthillMckeeOrdering.hppinclude/ordering/kingOrdering.hppinclude/ordering/sloanOrdering.hppinclude/ordering/topologicalSort.hpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.potsrc/ordering/CMakeLists.txtsrc/ordering/cuthillMckeeOrdering.csrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/cuthillMckeeOrdering_driver.cppsrc/ordering/kingOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/topologicalSort.cppsrc/ordering/topologicalSort_driver.cpp
💤 Files with no reviewable changes (5)
- include/drivers/ordering/topologicalSort_driver.h
- include/drivers/ordering/cuthillMckeeOrdering_driver.h
- src/ordering/topologicalSort_driver.cpp
- src/ordering/cuthillMckeeOrdering_driver.cpp
- include/c_types/i_rt.h
🧰 Additional context used
🧠 Learnings (40)
📓 Common learnings
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: NEWS.md:16-24
Timestamp: 2025-09-15T00:39:05.272Z
Learning: In pgRouting, editorial improvements to NEWS.md content should be implemented via the notes2news.pl script or its source files, since NEWS.md is auto-generated and manual edits would be overwritten.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_degree.rst:28-35
Timestamp: 2025-02-27T23:08:14.497Z
Learning: In the pgRouting documentation, historical version notes should be retained to show the evolution of functions across different versions.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: doc/contraction/pgr_contraction.rst:0-0
Timestamp: 2025-09-17T02:09:53.993Z
Learning: In pgRouting documentation, pseudocode blocks can use generic variable names that don't necessarily match the exact parameter names in the API, as decided by maintainer cvvergara for doc/contraction/pgr_contraction.rst.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2874
File: doc/utilities/pgr_separateCrossing.rst:26-27
Timestamp: 2025-04-25T15:12:03.608Z
Learning: In pgRouting documentation, official functions don't include version information in their index entries (`:single:` directives). Only deprecated functions include when they were deprecated in the index entries.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:130-144
Timestamp: 2025-01-19T13:16:52.140Z
Learning: In the pgRouting project, function documentation should be kept minimal and concise, focusing on essential information only.
📚 Learning: 2025-02-06T20:50:07.967Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rstinclude/ordering/cuthillMckeeOrdering.hppinclude/ordering/kingOrdering.hppinclude/ordering/topologicalSort.hppsrc/ordering/topologicalSort.clocale/pot/pgrouting_doc_strings.potinclude/ordering/sloanOrdering.hpplocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.c
📚 Learning: 2025-09-15T00:39:05.272Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: NEWS.md:16-24
Timestamp: 2025-09-15T00:39:05.272Z
Learning: In pgRouting, editorial improvements to NEWS.md content should be implemented via the notes2news.pl script or its source files, since NEWS.md is auto-generated and manual edits would be overwritten.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rstlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-27T01:49:25.212Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Applied to files:
NEWS.mddoc/src/release_notes.rstsrc/ordering/topologicalSort.clocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:41:25.296Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:56-57
Timestamp: 2025-04-22T22:41:25.296Z
Learning: In pgRouting, NEWS.md is an automatically generated file that should not be modified manually. Changes to the NEWS.md content should be made by modifying the generator script (tools/release-scripts/notes2news.pl) or its source files.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rstlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-06T20:50:00.577Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Applied to files:
NEWS.mddoc/src/release_notes.rstinclude/ordering/cuthillMckeeOrdering.hppinclude/ordering/kingOrdering.hppinclude/ordering/topologicalSort.hppsrc/ordering/topologicalSort.cinclude/ordering/sloanOrdering.hpp
📚 Learning: 2025-04-22T22:41:34.709Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:47-50
Timestamp: 2025-04-22T22:41:34.709Z
Learning: In pgRouting, NEWS.md is a generated file created by the tools/release-scripts/notes2news.pl script and should not be modified manually.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rstlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T23:08:14.497Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_degree.rst:28-35
Timestamp: 2025-02-27T23:08:14.497Z
Learning: In the pgRouting documentation, historical version notes should be retained to show the evolution of functions across different versions.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rstlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-24T23:34:23.099Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: NEWS.md:46-47
Timestamp: 2025-04-24T23:34:23.099Z
Learning: For pgRouting, NEWS.md should be updated using the generator script (tools/release-scripts/notes2news.pl) rather than edited directly, which is the correct procedure that the project maintainers follow.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rstlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-05-22T21:14:00.661Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2898
File: NEWS.md:99-99
Timestamp: 2025-05-22T21:14:00.661Z
Learning: In pgRouting, the NEWS.md file is automatically generated and should not be manually edited, as changes would be overwritten during the next generation.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rst
📚 Learning: 2025-02-27T23:09:12.162Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rstsrc/ordering/topologicalSort.c
📚 Learning: 2025-09-14T16:43:16.715Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/boost_version.yml:134-135
Timestamp: 2025-09-14T16:43:16.715Z
Learning: In pgrouting, the installed PostgreSQL extension version is normalized to major.minor.patch format (e.g., "4.0.0") regardless of any pre-release suffixes like -alpha1, -beta, or -rc that are used in the source/build versioning. Database setup scripts should use the normalized version format.
Applied to files:
NEWS.mddoc/src/release_notes.rstdoc/src/pgRouting-introduction.rst
📚 Learning: 2025-09-14T17:22:22.313Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/update.yml:132-133
Timestamp: 2025-09-14T17:22:22.313Z
Learning: In pgRouting, when testing extension updates in workflows, the target version should be the normalized major.minor.patch format (e.g., "4.0.0") even for pre-release versions like 4.0.0-alpha1, because the extension is installed with the normalized version number regardless of source version suffixes.
Applied to files:
NEWS.md
📚 Learning: 2025-01-19T16:19:16.974Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_withPoints_driver.cpp:154-154
Timestamp: 2025-01-19T16:19:16.974Z
Learning: Error messages in pgRouting should be kept minimal and consistent to maintain compatibility with pgtap tests.
Applied to files:
NEWS.mddoc/src/release_notes.rstsrc/ordering/ordering_driver.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:53.496Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/coloring/edgeColoring_driver.cpp:87-87
Timestamp: 2025-01-19T16:17:53.496Z
Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.
Applied to files:
NEWS.mdlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-25T15:12:03.608Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2874
File: doc/utilities/pgr_separateCrossing.rst:26-27
Timestamp: 2025-04-25T15:12:03.608Z
Learning: In pgRouting documentation, official functions don't include version information in their index entries (`:single:` directives). Only deprecated functions include when they were deprecated in the index entries.
Applied to files:
doc/src/release_notes.rst
📚 Learning: 2025-02-27T16:42:23.807Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: doc/withPoints/pgr_withPoints.rst:30-35
Timestamp: 2025-02-27T16:42:23.807Z
Learning: For pgr_withPoints, the original function was proposed in version 2.2.0, while the Combinations signature overload was added in version a3.2.0.
Applied to files:
doc/src/release_notes.rst
📚 Learning: 2025-02-27T16:34:57.518Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: sql/sigs/pgrouting--3.8.sig:274-275
Timestamp: 2025-02-27T16:34:57.518Z
Learning: The file `sql/sigs/pgrouting--3.8.sig` is auto-generated by a command and should not be modified manually.
Applied to files:
doc/src/pgRouting-introduction.rst
📚 Learning: 2025-09-14T17:29:04.360Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: CMakeLists.txt:96-104
Timestamp: 2025-09-14T17:29:04.360Z
Learning: In pgRouting, BOOST_THREAD_LIBRARIES is referenced in CMakeLists.txt but the project doesn't actually use Boost threading functionality in the source code, so the LINK_LIBRARIES(${BOOST_THREAD_LIBRARIES}) line should be removed as unused legacy code.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppinclude/ordering/cuthillMckeeOrdering.hppinclude/ordering/sloanOrdering.hpp
📚 Learning: 2025-04-25T00:59:52.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Applied to files:
src/ordering/topologicalSort.csrc/ordering/ordering_driver.cpp
📚 Learning: 2025-04-29T18:59:06.824Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2879
File: sql/dijkstra/dijkstra.sql:56-57
Timestamp: 2025-04-29T18:59:06.824Z
Learning: The maintainer prefers not to add inline comments to internal function calls like _pgr_dijkstra, even when they contain multiple boolean and numeric flags.
Applied to files:
src/ordering/topologicalSort.c
📚 Learning: 2025-04-24T23:29:43.790Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:18:55.529Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/dijkstra/dijkstra_driver.cpp:147-148
Timestamp: 2025-01-19T16:18:55.529Z
Learning: In pgRouting, error messages should not be modified without careful consideration as they are verified by pgtap tests. Changes to error messages require corresponding updates to test cases.
Applied to files:
src/ordering/ordering_driver.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:38.435Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/pickDeliver/pickDeliver_driver.cpp:69-69
Timestamp: 2025-01-19T16:17:38.435Z
Learning: Error messages in pgRouting are part of pgtap test assertions and should not be modified without updating the corresponding tests.
Applied to files:
src/ordering/ordering_driver.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:18:09.359Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_driver.cpp:231-232
Timestamp: 2025-01-19T16:18:09.359Z
Learning: In pgRouting, modifications to error message handling need careful consideration as they impact pgtap test cases which verify exact error message content.
Applied to files:
src/ordering/ordering_driver.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:18:00.783Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_withPoints_driver.cpp:120-121
Timestamp: 2025-01-19T16:18:00.783Z
Learning: In pgRouting, modifying error messages requires updating pgtap test expectations, so such changes should be carefully considered due to the test maintenance burden.
Applied to files:
src/ordering/ordering_driver.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:31.895Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/contraction/contractGraph_driver.cpp:187-187
Timestamp: 2025-01-19T16:17:31.895Z
Learning: Changes to error messages in pgRouting must carefully consider impacts on pgtap tests, as these tests verify exact message content.
Applied to files:
src/ordering/ordering_driver.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:42.135Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_driver.cpp:151-151
Timestamp: 2025-01-19T16:17:42.135Z
Learning: Modifying error messages in pgRouting requires updating the corresponding pgtap tests to match the new messages, as they verify exact string outputs.
Applied to files:
src/ordering/ordering_driver.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:46.713Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/planar/boyerMyrvold_driver.cpp:100-100
Timestamp: 2025-01-19T16:17:46.713Z
Learning: Changes to error message handling in pgRouting need to consider impact on pgtap tests, as they may have assertions that depend on specific message behavior.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-12-31T06:01:45.144Z
Learnt from: Mohit242-bit
Repo: pgRouting/pgrouting PR: 3018
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:11-11
Timestamp: 2025-12-31T06:01:45.144Z
Learning: In pgRouting/pgrouting, CI/Weblate may update locale/pot/pgrouting_doc_strings.pot and locale/en/LC_MESSAGES/pgrouting_doc_strings.po in PRs after .rst changes. These auto-generated updates are acceptable and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-10-30T16:43:11.554Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2989
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:10-10
Timestamp: 2025-10-30T16:43:11.554Z
Learning: When a locale translation change (e.g., locale/es/LC_MESSAGES/pgrouting_doc_strings.po) is introduced by a Weblate-authored commit within a PR, it is compliant with pgRouting’s workflow and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T16:32:00.573Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:0-0
Timestamp: 2025-02-27T16:32:00.573Z
Learning: Modifications to locale files in the pgRouting project should be handled through an action commit or through the Weblate translation platform, not via direct edits.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T16:31:49.972Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:2740-2740
Timestamp: 2025-02-27T16:31:49.972Z
Learning: Modifications to locale files in pgRouting (like .po files) are done through action commits or in weblate, not through direct manual edits in PRs.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T23:03:08.160Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:4407-4419
Timestamp: 2025-02-27T23:03:08.160Z
Learning: In the pgRouting project, locale files (.po files) should not be manually edited as they are managed through automated localization tools.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:59:10.377Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:15446-15454
Timestamp: 2025-04-22T22:59:10.377Z
Learning: En pgRouting, las traducciones al español de las cadenas de documentación (por ejemplo, en `pgrouting_doc_strings.po`) se gestionan a través de Weblate y no se deben sugerir ni modificar directamente en los pull requests.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:58:08.854Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:16304-16305
Timestamp: 2025-04-22T22:58:08.854Z
Learning: En el repositorio pgRouting, las traducciones al español se gestionan exclusivamente a través de Weblate, por lo que no se deben sugerir cambios manuales en los archivos .po para ese idioma en las revisiones de PR.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-09-14T23:32:11.977Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: doc/CMakeLists.txt:160-161
Timestamp: 2025-09-14T23:32:11.977Z
Learning: The pgRouting project uses Weblate for translation management. The CMake locale target only generates English locale files (`-l en`) because Weblate handles updating all other language translations automatically. This prevents conflicts between local builds and Weblate's translation workflow.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:36:50.711Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/zh_Hans/LC_MESSAGES/pgrouting_doc_strings.po:4083-4087
Timestamp: 2025-04-22T22:36:50.711Z
Learning: For the `locale/zh_Hans/LC_MESSAGES/pgrouting_doc_strings.po` file, Chinese translations should be managed and updated via Weblate, not by direct manual edits or suggestions in the repository.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:58:42.373Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:4429-4443
Timestamp: 2025-04-22T22:58:42.373Z
Learning: En el proyecto pgRouting, las traducciones al español se gestionan exclusivamente a través de Weblate, por lo que no se deben sugerir cambios de traducción en los pull requests.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T23:05:28.589Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: sql/common/_checkquery.sql:33-33
Timestamp: 2025-02-27T23:05:28.589Z
Learning: When reviewing PostgreSQL/pgRouting code, respect the maintainer's preference for brevity over additional comments when variable names are descriptive.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
🧬 Code graph analysis (4)
src/ordering/sloanOrdering.cpp (2)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)
src/ordering/cuthillMckeeOrdering.cpp (2)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)
src/ordering/kingOrdering.cpp (2)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)
src/ordering/ordering_driver.cpp (2)
include/ordering/sloanOrdering.hpp (1)
sloanOrdering(40-40)include/ordering/kingOrdering.hpp (1)
kingOrdering(41-41)
🪛 Cppcheck (2.19.0)
src/ordering/topologicalSort.cpp
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'topologicalSort' is never used.
(unusedFunction)
src/ordering/sloanOrdering.cpp
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'sloanOrdering' is never used.
(unusedFunction)
src/ordering/cuthillMckeeOrdering.cpp
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 58-58: The function 'cuthillMckeeOrdering' is never used.
(unusedFunction)
src/ordering/kingOrdering.cpp
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'kingOrdering' is never used.
(unusedFunction)
src/ordering/topologicalSort.c
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 43-43: The function '_pgr_topologicalsort' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
src/ordering/ordering_driver.cpp
[information] 40-40: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 45-45: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[style] 57-57: The function 'set_ids' is never used.
(unusedFunction)
[style] 59-59: The function 'dest_id' is never used.
(unusedFunction)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[performance] 113-113: Function parameter 'results' should be passed by const reference.
(passedByValue)
[style] 111-111: The function 'vector_to_tuple' is never used.
(unusedFunction)
src/ordering/cuthillMckeeOrdering.c
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 42-42: The function '_pgr_cuthillmckeeordering' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
🔇 Additional comments (20)
doc/src/pgRouting-introduction.rst (1)
66-66: LGTM! Good use of release placeholder.Replacing the hardcoded version with the
|release|placeholder improves maintainability and ensures the documentation automatically reflects the current release version.include/ordering/cuthillMckeeOrdering.hpp (1)
40-41: Parameter cannot be const due to Boost API constraints.While the implementation only reads the graph and doesn't modify it, the
graphparameter must remain a non-const reference becauseboost::cuthill_mckee_ordering(the underlying Boost library function) requires a non-const Graph reference. This is consistent with other ordering functions in the codebase (sloanOrdering, kingOrdering), which face the same Boost API limitation.Likely an incorrect or invalid review comment.
locale/en/LC_MESSAGES/pgrouting_doc_strings.po (4)
11-11: Auto‑updated POT creation date – no action needed
POT-Creation-Datechange is generated by the docs/locale tooling and is expected; nothing to review here.
8326-8330: Per‑release contributors heading and list look correctUsing
|release|in “Individuals in this release |release| (in alphabetical order)” makes the string reusable across versions, and restricting the per‑release names to “Mohit Rawat, Regina Obe, Vicky Vergara” matches the 4.1.0 release context. No changes needed.
8392-8402: Contributors list reflow is purely formattingThe multi‑line
msgidfor “Contributors Past & Present” is just rewrapped; the actual list of names is unchanged. This is a no‑op from a documentation standpoint.
16167-16174: New 4.1.0 “Code enhancement” entry correctly documents ordering changesThe added “Code enhancement” heading and the note
#3025 …: Ordering functions standardizedaccurately describe the standardization work on ordering functions implemented in this PR and are placed under the 4.1.0 release section appropriately. Wording is clear and consistent with other release‑notes bullets.
locale/pot/pgrouting_doc_strings.pot (1)
11-11: Auto-generated locale updates look correctThe updated POT creation date, contributor strings, and the new “Code enhancement” entry for
#3025: Ordering functions standardizedare consistent with the PR goal and the surrounding release-notes structure. Given that this.potfile is generated by the docs/locale tooling and/or Weblate, these changes should be accepted as-is without manual adjustment.Also applies to: 7147-7151, 13510-13514
include/ordering/kingOrdering.hpp (1)
37-44: LGTM! Clean function declaration.The kingOrdering function declaration correctly follows the standardization approach, moving from template-based to a concrete free function in the pgrouting::functions namespace. This aligns with the PR objectives to remove unnecessary templates and consolidate ordering functions.
NEWS.md (1)
19-22: Verify NEWS.md is generated, not manually edited.Based on learnings, NEWS.md is automatically generated by the
tools/release-scripts/notes2news.plscript and should not be manually edited. Please confirm this entry was added through the proper generation process rather than manual edits, as manual changes would be overwritten during the next generation.doc/src/release_notes.rst (1)
49-52: LGTM! Documentation properly updated.The code enhancement section is correctly added with the appropriate issue reference. This documentation change aligns with the PR objectives to standardize ordering functions.
src/ordering/kingOrdering.cpp (1)
46-84: LGTM! Solid implementation of King ordering.The implementation correctly:
- Initializes all required data structures (index map, permutation vector, color map, degree map)
- Includes interruption checks for long-running operations
- Properly invokes boost::king_ordering with reverse iterators to get the correct ordering
- Handles exceptions appropriately
src/ordering/sloanOrdering.cpp (1)
46-89: LGTM!The implementation correctly uses Boost's Sloan ordering algorithm with proper interrupt checks and exception handling. The function properly initializes all required property maps and returns the computed permutation.
src/ordering/cuthillMckeeOrdering.cpp (1)
57-95: LGTM!The implementation correctly uses Boost's Cuthill-McKee ordering with
rbegin()to produce the reverse ordering. Proper interrupt checks and exception handling are in place.src/ordering/cuthillMckeeOrdering.c (1)
46-96: LGTM!The changes correctly integrate with the new process-based architecture. The result type change from
II_t_rt*toint64_t*and the reduction from 3 to 2 output columns align with the standardization objectives. The SRF plumbing is properly updated.include/ordering/topologicalSort.hpp (1)
31-47: LGTM!The simplified header cleanly exposes the topological sort as a free function in the
pgrouting::functionsnamespace. The API is clear, and the use ofconst DirectedGraph&properly indicates the function doesn't modify the graph.src/ordering/CMakeLists.txt (1)
1-14: LGTM!The build configuration correctly adds the four new implementation files (cuthillMckeeOrdering.cpp, kingOrdering.cpp, sloanOrdering.cpp, topologicalSort.cpp) to replace the removed driver-based sources. This aligns with the PR's standardization objectives.
include/ordering/sloanOrdering.hpp (1)
30-45: LGTM! Clean header refactoring.The conversion from a templated class-based API to a free function in the
pgrouting::functionsnamespace is well-executed. The function signature is consistent with other ordering functions (kingOrdering,cuthillMckeeOrdering) as shown in the relevant code snippets.src/ordering/ordering_driver.cpp (2)
51-60: Good abstraction with the templated process helper.The
processtemplate cleanly encapsulates the common pattern of edge insertion, ordering execution, and result conversion. This reduces code duplication across the different ordering algorithms.
62-106: LGTM! Clean driver implementation.The refactored
do_orderingfunction with switch-based dispatch is cleaner than the previous if-cascade. The consolidation of ordering functions into a single driver aligns well with the PR objectives.src/ordering/topologicalSort.c (1)
79-102: LGTM! Clean SRF implementation.The set-returning function lifecycle is correctly implemented. The simplification from a struct-based result type to a direct
int64_tarray is appropriate given the two-column output (sequence number and vertex id).Note: The static analysis hint about static linkage is a false positive—
PGDLLEXPORTfunctions must have external linkage to be callable from PostgreSQL.
cvvergara
force-pushed
the
standardize-ordering-functions-code
branch
from
bdf72d8 to
dfc6256
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ordering/ordering_driver.cpp (1)
153-155: Inverted ternary logic for log message.The condition on line 155 appears inverted. When
hint.empty()is true, you're converting the empty hint; when it's false (hint has content), you're converting the log. This should likely be the opposite - use the log when hint is empty, and use hint when it has content.🔎 Proposed fix
} catch (const std::string &ex) { *err_msg = to_pg_msg(ex); - *log_msg = hint.empty()? to_pg_msg(hint) : to_pg_msg(log); + *log_msg = hint.empty()? to_pg_msg(log) : to_pg_msg(hint); }
🤖 Fix all issues with AI Agents
In @src/ordering/cuthillMckeeOrdering.cpp:
- Around line 30-40: The implementation file cuthillMckeeOrdering.cpp is missing
its corresponding header include; add #include
"ordering/cuthillMckeeOrdering.hpp" near the top of cuthillMckeeOrdering.cpp (as
kingOrdering.cpp does) so the declarations and definitions for the Cuthill–McKee
functions/classes remain synchronized and to prevent ODR/compilation issues.
In @src/ordering/ordering_driver.cpp:
- Around line 110-115: Remove the stray extra semicolon after the DirectedGraph
construction and fix the vertex-extraction condition: currently vertices are
only extracted when which == 0 or which == 2, but algorithms referenced by which
== 1 (Cuthill–McKee) and which == 11 (topological sort / directed graph) may
also need pre-extracted vertices. Update the ternary that initializes vertices
(the expression using pgrouting::extract_vertices(edges)) to include which == 1
and which == 11 if those algorithms require vertices, and then construct
UndirectedGraph(vertices) or DirectedGraph(vertices) accordingly; finally delete
the redundant semicolon after DirectedGraph() so only a single semicolon
terminates the statement.
- Around line 118-135: The switch on variable which in ordering_driver.cpp
(cases 0,1,2,11) lacks a default branch so unexpected values silently yield no
results; add a default case that logs or throws an error and sets a clear
failure state before returning (e.g., use processLogger/error, throw
std::invalid_argument, or set *return_count to -1) to indicate a programming
error; update the switch (around the existing cases invoking process(edges,...,
*return_count, return_tuples)) to include this default that references which and
ensures callers can detect the invalid input.
In @src/ordering/sloanOrdering.cpp:
- Around line 46-89: In sloanOrdering, replace the direct call
boost::num_vertices(graph.graph) with the wrapper method graph.num_vertices() to
follow project conventions: update the size_t n initialization in function
sloanOrdering to call graph.num_vertices(), leaving all other property-map setup
and exception handling unchanged so behavior is preserved.
In @src/ordering/topologicalSort.cpp:
- Around line 55-64: The catch block that currently does "throw
std::string(\"Graph is not DAG\")" should throw a proper exception type; replace
that with throwing std::runtime_error with the same message (e.g. throw
std::runtime_error("Graph is not DAG")); ensure <stdexcept> is included if not
already; keep the existing catch clauses and propagation for other exception
types (the try around boost::topological_sort on graph.graph writing into
results) unchanged.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (23)
NEWS.mddoc/src/pgRouting-introduction.rstdoc/src/release_notes.rstinclude/c_types/i_rt.hinclude/cpp_common/to_postgres.hppinclude/drivers/ordering/cuthillMckeeOrdering_driver.hinclude/drivers/ordering/topologicalSort_driver.hinclude/ordering/cuthillMckeeOrdering.hppinclude/ordering/kingOrdering.hppinclude/ordering/sloanOrdering.hppinclude/ordering/topologicalSort.hpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.potsrc/ordering/CMakeLists.txtsrc/ordering/cuthillMckeeOrdering.csrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/cuthillMckeeOrdering_driver.cppsrc/ordering/kingOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/topologicalSort.cppsrc/ordering/topologicalSort_driver.cpp
💤 Files with no reviewable changes (5)
- include/c_types/i_rt.h
- include/drivers/ordering/topologicalSort_driver.h
- include/drivers/ordering/cuthillMckeeOrdering_driver.h
- src/ordering/topologicalSort_driver.cpp
- src/ordering/cuthillMckeeOrdering_driver.cpp
🧰 Additional context used
🧠 Learnings (44)
📓 Common learnings
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: NEWS.md:16-24
Timestamp: 2025-09-15T00:39:05.272Z
Learning: In pgRouting, editorial improvements to NEWS.md content should be implemented via the notes2news.pl script or its source files, since NEWS.md is auto-generated and manual edits would be overwritten.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2871
File: include/drivers/transitiveClosure/transitiveClosure_driver.h:45-49
Timestamp: 2025-04-25T02:12:17.271Z
Learning: The pgRouting project avoids using named parameters in C header files to prevent conflicts with Doxygen documentation.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_degree.rst:28-35
Timestamp: 2025-02-27T23:08:14.497Z
Learning: In the pgRouting documentation, historical version notes should be retained to show the evolution of functions across different versions.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:130-144
Timestamp: 2025-01-19T13:16:52.140Z
Learning: In the pgRouting project, function documentation should be kept minimal and concise, focusing on essential information only.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2874
File: doc/utilities/pgr_separateCrossing.rst:26-27
Timestamp: 2025-04-25T15:12:03.608Z
Learning: In pgRouting documentation, official functions don't include version information in their index entries (`:single:` directives). Only deprecated functions include when they were deprecated in the index entries.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
📚 Learning: 2025-04-22T22:41:25.296Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:56-57
Timestamp: 2025-04-22T22:41:25.296Z
Learning: In pgRouting, NEWS.md is an automatically generated file that should not be modified manually. Changes to the NEWS.md content should be made by modifying the generator script (tools/release-scripts/notes2news.pl) or its source files.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.mdlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-24T23:34:23.099Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: NEWS.md:46-47
Timestamp: 2025-04-24T23:34:23.099Z
Learning: For pgRouting, NEWS.md should be updated using the generator script (tools/release-scripts/notes2news.pl) rather than edited directly, which is the correct procedure that the project maintainers follow.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.mdlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-22T22:41:34.709Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:47-50
Timestamp: 2025-04-22T22:41:34.709Z
Learning: In pgRouting, NEWS.md is a generated file created by the tools/release-scripts/notes2news.pl script and should not be modified manually.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.mdlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-09-15T00:39:05.272Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: NEWS.md:16-24
Timestamp: 2025-09-15T00:39:05.272Z
Learning: In pgRouting, editorial improvements to NEWS.md content should be implemented via the notes2news.pl script or its source files, since NEWS.md is auto-generated and manual edits would be overwritten.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.mdlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-05-22T21:14:00.661Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2898
File: NEWS.md:99-99
Timestamp: 2025-05-22T21:14:00.661Z
Learning: In pgRouting, the NEWS.md file is automatically generated and should not be manually edited, as changes would be overwritten during the next generation.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.mdlocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T23:08:14.497Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_degree.rst:28-35
Timestamp: 2025-02-27T23:08:14.497Z
Learning: In the pgRouting documentation, historical version notes should be retained to show the evolution of functions across different versions.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.mdlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-06T20:50:07.967Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.mdsrc/ordering/cuthillMckeeOrdering.csrc/ordering/topologicalSort.cinclude/ordering/topologicalSort.hppinclude/ordering/sloanOrdering.hpplocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T23:09:12.162Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rstNEWS.md
📚 Learning: 2025-02-27T16:34:57.518Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: sql/sigs/pgrouting--3.8.sig:274-275
Timestamp: 2025-02-27T16:34:57.518Z
Learning: The file `sql/sigs/pgrouting--3.8.sig` is auto-generated by a command and should not be modified manually.
Applied to files:
doc/src/pgRouting-introduction.rstsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-04-25T15:12:03.608Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2874
File: doc/utilities/pgr_separateCrossing.rst:26-27
Timestamp: 2025-04-25T15:12:03.608Z
Learning: In pgRouting documentation, official functions don't include version information in their index entries (`:single:` directives). Only deprecated functions include when they were deprecated in the index entries.
Applied to files:
doc/src/pgRouting-introduction.rstdoc/src/release_notes.rst
📚 Learning: 2025-01-27T01:49:25.212Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Applied to files:
doc/src/release_notes.rstNEWS.mdsrc/ordering/topologicalSort.clocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-06T20:50:00.577Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Applied to files:
doc/src/release_notes.rstNEWS.mdinclude/ordering/kingOrdering.hppsrc/ordering/topologicalSort.cinclude/ordering/topologicalSort.hppinclude/ordering/sloanOrdering.hpp
📚 Learning: 2025-02-27T16:42:23.807Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: doc/withPoints/pgr_withPoints.rst:30-35
Timestamp: 2025-02-27T16:42:23.807Z
Learning: For pgr_withPoints, the original function was proposed in version 2.2.0, while the Combinations signature overload was added in version a3.2.0.
Applied to files:
doc/src/release_notes.rst
📚 Learning: 2025-09-14T16:43:16.715Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/boost_version.yml:134-135
Timestamp: 2025-09-14T16:43:16.715Z
Learning: In pgrouting, the installed PostgreSQL extension version is normalized to major.minor.patch format (e.g., "4.0.0") regardless of any pre-release suffixes like -alpha1, -beta, or -rc that are used in the source/build versioning. Database setup scripts should use the normalized version format.
Applied to files:
doc/src/release_notes.rstNEWS.md
📚 Learning: 2025-01-19T16:19:16.974Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_withPoints_driver.cpp:154-154
Timestamp: 2025-01-19T16:19:16.974Z
Learning: Error messages in pgRouting should be kept minimal and consistent to maintain compatibility with pgtap tests.
Applied to files:
doc/src/release_notes.rstNEWS.mdsrc/ordering/ordering_driver.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:18:49.447Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/bellman_ford/bellman_ford_neg_driver.cpp:119-119
Timestamp: 2025-01-19T16:18:49.447Z
Learning: The edge count check in bellman_ford_neg_driver.cpp is part of code that is not yet in use, as of January 2025.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-09-14T17:22:22.313Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/update.yml:132-133
Timestamp: 2025-09-14T17:22:22.313Z
Learning: In pgRouting, when testing extension updates in workflows, the target version should be the normalized major.minor.patch format (e.g., "4.0.0") even for pre-release versions like 4.0.0-alpha1, because the extension is installed with the normalized version number regardless of source version suffixes.
Applied to files:
NEWS.md
📚 Learning: 2025-01-19T16:17:53.496Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/coloring/edgeColoring_driver.cpp:87-87
Timestamp: 2025-01-19T16:17:53.496Z
Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.
Applied to files:
NEWS.mdlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T23:05:28.589Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: sql/common/_checkquery.sql:33-33
Timestamp: 2025-02-27T23:05:28.589Z
Learning: When reviewing PostgreSQL/pgRouting code, respect the maintainer's preference for brevity over additional comments when variable names are descriptive.
Applied to files:
src/ordering/cuthillMckeeOrdering.csrc/ordering/sloanOrdering.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T16:33:56.959Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: sql/sigs/pgrouting--3.8.sig:125-136
Timestamp: 2025-02-27T16:33:56.959Z
Learning: The file sql/sigs/pgrouting--*.sig is automatically generated by a command and cannot be manually modified. Comments about naming conventions or other improvements should be directed at the source code that generates these signatures, not at the signature file itself.
Applied to files:
src/ordering/sloanOrdering.cpp
📚 Learning: 2025-04-24T23:36:04.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: include/drivers/contraction/contractionHierarchies_driver.h:1-73
Timestamp: 2025-04-24T23:36:04.580Z
Learning: The copyright year for pgRouting should be set to 2025 in new files, not 2024, as confirmed by the project maintainer.
Applied to files:
src/ordering/sloanOrdering.cpp
📚 Learning: 2025-04-25T00:59:52.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Applied to files:
src/ordering/topologicalSort.csrc/ordering/ordering_driver.cpp
📚 Learning: 2025-04-29T18:59:06.824Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2879
File: sql/dijkstra/dijkstra.sql:56-57
Timestamp: 2025-04-29T18:59:06.824Z
Learning: The maintainer prefers not to add inline comments to internal function calls like _pgr_dijkstra, even when they contain multiple boolean and numeric flags.
Applied to files:
src/ordering/topologicalSort.c
📚 Learning: 2025-12-30T19:45:32.986Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3004
File: .clang-tidy:9-11
Timestamp: 2025-12-30T19:45:32.986Z
Learning: In pgRouting's .clang-tidy file (and similar list configurations), the last line of lists should not have a trailing comma, per project convention.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-12-30T19:45:48.714Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3004
File: .clang-tidy:11-11
Timestamp: 2025-12-30T19:45:48.714Z
Learning: In pgRouting, the last line of lists (such as in .clang-tidy configuration files) does not have a trailing comma, following the project's coding convention.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-04-24T23:29:43.790Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:18:55.529Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/dijkstra/dijkstra_driver.cpp:147-148
Timestamp: 2025-01-19T16:18:55.529Z
Learning: In pgRouting, error messages should not be modified without careful consideration as they are verified by pgtap tests. Changes to error messages require corresponding updates to test cases.
Applied to files:
src/ordering/ordering_driver.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:17:38.435Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/pickDeliver/pickDeliver_driver.cpp:69-69
Timestamp: 2025-01-19T16:17:38.435Z
Learning: Error messages in pgRouting are part of pgtap test assertions and should not be modified without updating the corresponding tests.
Applied to files:
src/ordering/ordering_driver.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:18:09.359Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_driver.cpp:231-232
Timestamp: 2025-01-19T16:18:09.359Z
Learning: In pgRouting, modifications to error message handling need careful consideration as they impact pgtap test cases which verify exact error message content.
Applied to files:
src/ordering/ordering_driver.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:18:00.783Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_withPoints_driver.cpp:120-121
Timestamp: 2025-01-19T16:18:00.783Z
Learning: In pgRouting, modifying error messages requires updating pgtap test expectations, so such changes should be carefully considered due to the test maintenance burden.
Applied to files:
src/ordering/ordering_driver.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:17:31.895Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/contraction/contractGraph_driver.cpp:187-187
Timestamp: 2025-01-19T16:17:31.895Z
Learning: Changes to error messages in pgRouting must carefully consider impacts on pgtap tests, as these tests verify exact message content.
Applied to files:
src/ordering/ordering_driver.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:17:42.135Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_driver.cpp:151-151
Timestamp: 2025-01-19T16:17:42.135Z
Learning: Modifying error messages in pgRouting requires updating the corresponding pgtap tests to match the new messages, as they verify exact string outputs.
Applied to files:
src/ordering/ordering_driver.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:17:46.713Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/planar/boyerMyrvold_driver.cpp:100-100
Timestamp: 2025-01-19T16:17:46.713Z
Learning: Changes to error message handling in pgRouting need to consider impact on pgtap tests, as they may have assertions that depend on specific message behavior.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-12-31T06:01:45.144Z
Learnt from: Mohit242-bit
Repo: pgRouting/pgrouting PR: 3018
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:11-11
Timestamp: 2025-12-31T06:01:45.144Z
Learning: In pgRouting/pgrouting, CI/Weblate may update locale/pot/pgrouting_doc_strings.pot and locale/en/LC_MESSAGES/pgrouting_doc_strings.po in PRs after .rst changes. These auto-generated updates are acceptable and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T16:31:49.972Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:2740-2740
Timestamp: 2025-02-27T16:31:49.972Z
Learning: Modifications to locale files in pgRouting (like .po files) are done through action commits or in weblate, not through direct manual edits in PRs.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T16:32:00.573Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:0-0
Timestamp: 2025-02-27T16:32:00.573Z
Learning: Modifications to locale files in the pgRouting project should be handled through an action commit or through the Weblate translation platform, not via direct edits.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-10-30T16:43:11.554Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2989
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:10-10
Timestamp: 2025-10-30T16:43:11.554Z
Learning: When a locale translation change (e.g., locale/es/LC_MESSAGES/pgrouting_doc_strings.po) is introduced by a Weblate-authored commit within a PR, it is compliant with pgRouting’s workflow and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T23:03:08.160Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:4407-4419
Timestamp: 2025-02-27T23:03:08.160Z
Learning: In the pgRouting project, locale files (.po files) should not be manually edited as they are managed through automated localization tools.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-22T22:59:10.377Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:15446-15454
Timestamp: 2025-04-22T22:59:10.377Z
Learning: En pgRouting, las traducciones al español de las cadenas de documentación (por ejemplo, en `pgrouting_doc_strings.po`) se gestionan a través de Weblate y no se deben sugerir ni modificar directamente en los pull requests.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-22T22:58:08.854Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:16304-16305
Timestamp: 2025-04-22T22:58:08.854Z
Learning: En el repositorio pgRouting, las traducciones al español se gestionan exclusivamente a través de Weblate, por lo que no se deben sugerir cambios manuales en los archivos .po para ese idioma en las revisiones de PR.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-09-14T23:32:11.977Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: doc/CMakeLists.txt:160-161
Timestamp: 2025-09-14T23:32:11.977Z
Learning: The pgRouting project uses Weblate for translation management. The CMake locale target only generates English locale files (`-l en`) because Weblate handles updating all other language translations automatically. This prevents conflicts between local builds and Weblate's translation workflow.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-22T22:36:50.711Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/zh_Hans/LC_MESSAGES/pgrouting_doc_strings.po:4083-4087
Timestamp: 2025-04-22T22:36:50.711Z
Learning: For the `locale/zh_Hans/LC_MESSAGES/pgrouting_doc_strings.po` file, Chinese translations should be managed and updated via Weblate, not by direct manual edits or suggestions in the repository.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-22T22:58:42.373Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:4429-4443
Timestamp: 2025-04-22T22:58:42.373Z
Learning: En el proyecto pgRouting, las traducciones al español se gestionan exclusivamente a través de Weblate, por lo que no se deben sugerir cambios de traducción en los pull requests.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
🧬 Code graph analysis (4)
src/ordering/cuthillMckeeOrdering.cpp (2)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)
src/ordering/kingOrdering.cpp (2)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)
src/ordering/sloanOrdering.cpp (2)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)
src/ordering/ordering_driver.cpp (2)
include/ordering/sloanOrdering.hpp (1)
sloanOrdering(41-41)include/ordering/kingOrdering.hpp (1)
kingOrdering(42-42)
🪛 Cppcheck (2.19.0)
src/ordering/cuthillMckeeOrdering.cpp
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 58-58: The function 'cuthillMckeeOrdering' is never used.
(unusedFunction)
src/ordering/kingOrdering.cpp
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'kingOrdering' is never used.
(unusedFunction)
src/ordering/cuthillMckeeOrdering.c
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 42-42: The function '_pgr_cuthillmckeeordering' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
src/ordering/sloanOrdering.cpp
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'sloanOrdering' is never used.
(unusedFunction)
src/ordering/topologicalSort.c
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 43-43: The function '_pgr_topologicalsort' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
src/ordering/topologicalSort.cpp
[information] 35-35: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 48-48: The function 'topologicalSort' is never used.
(unusedFunction)
src/ordering/ordering_driver.cpp
[information] 40-40: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 45-45: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[style] 57-57: The function 'set_ids' is never used.
(unusedFunction)
[style] 59-59: The function 'dest_id' is never used.
(unusedFunction)
[performance] 78-78: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[performance] 113-113: Function parameter 'results' should be passed by const reference.
(passedByValue)
[style] 111-111: The function 'vector_to_tuple' is never used.
(unusedFunction)
🔇 Additional comments (23)
src/ordering/CMakeLists.txt (1)
9-13: LGTM!The build configuration correctly reflects the architectural refactoring, replacing driver sources with dedicated implementation files for each ordering algorithm. This aligns with the PR's objective to standardize ordering functions.
include/ordering/cuthillMckeeOrdering.hpp (1)
41-42: LGTM!The free function interface is clean and appropriately scoped. The use of
UndirectedGraphspecifically (rather than a template) correctly reflects that Cuthill-McKee ordering works only for undirected graphs, as stated in the PR objectives.src/ordering/cuthillMckeeOrdering.c (1)
35-96: LGTM!The changes correctly adapt the PostgreSQL SRF wrapper to the new unified processing architecture:
- Include updated to use the new process header
- Result type simplified to
int64_t*for direct vertex ID output- Column count reduced from 3 to 2 (sequence and vertex_id)
- Per-call data handling properly adjusted for the new format
The implementation follows standard PostgreSQL SRF patterns and correctly integrates with the refactored ordering process.
src/ordering/cuthillMckeeOrdering.cpp (1)
57-95: LGTM!The Cuthill-McKee ordering implementation is correct. The use of
boost::make_out_degree_mapis appropriate for this algorithm, and the property map initialization follows Boost Graph Library patterns.src/ordering/kingOrdering.cpp (2)
30-42: LGTM!The header organization is correct - the implementation file properly includes its corresponding header first, followed by standard library and Boost headers.
46-84: LGTM!The King ordering implementation is correct. The function properly initializes the required property maps and correctly uses the 5-parameter overload of
boost::king_orderingwhich requires the explicitindex_mapparameter.src/ordering/topologicalSort.cpp (1)
47-68: Good use of const reference for the graph parameter.The function correctly takes the graph by
constreference sinceboost::topological_sortdoesn't modify the graph structure. The reversal at line 66 is necessary because Boost'stopological_sortoutputs vertices in reverse topological order.include/ordering/sloanOrdering.hpp (1)
37-44: LGTM!The header correctly declares the
sloanOrderingfunction in thepgrouting::functionsnamespace, consistent with the other ordering function headers in this PR.include/ordering/topologicalSort.hpp (1)
39-46: LGTM!The header correctly declares
topologicalSortwith aconst DirectedGraph¶meter, appropriately reflecting that topological sort operates on directed graphs and doesn't modify the graph structure.src/ordering/ordering_driver.cpp (1)
51-60: LGTM!The
processtemplate function provides a clean, reusable abstraction for executing ordering algorithms. It correctly handles edge insertion, algorithm execution, and result conversion.include/ordering/kingOrdering.hpp (1)
38-45: LGTM!The header correctly declares the
kingOrderingfunction in thepgrouting::functionsnamespace with anUndirectedGraph¶meter, consistent with the other undirected graph ordering functions.src/ordering/topologicalSort.c (4)
36-37: LGTM! Include changes align with standardization.The replacement of driver-specific headers with the unified
process/ordering_process.hcorrectly reflects the consolidation of ordering functions into a single processing pathway.
47-47: LGTM! Type refactoring is consistent.The change from
II_t_rt*toint64_t*simplifies the data structure by havingpgr_process_orderingreturn just the ordered vertex IDs, with sequence numbers generated locally. The type usage is consistent throughout (declaration, cast, and datum conversion).Also applies to: 77-77, 94-95
85-92: LGTM! Variable naming improved.The rename from
numbtonumenhances readability, and the loop correctly initializes both array elements.
55-59: Magic number concern from previous review still applies.The past review comment about replacing magic number
11with a named constant remains valid and applicable. This would improve maintainability and consistency with the ordering driver.⛔ Skipped due to learnings
Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2771 File: sql/common/_checkquery.sql:33-33 Timestamp: 2025-02-27T23:05:28.589Z Learning: When reviewing PostgreSQL/pgRouting code, respect the maintainer's preference for brevity over additional comments when variable names are descriptive.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2868 File: src/contraction/contractionHierarchies_driver.cpp:47-56 Timestamp: 2025-04-25T00:59:52.580Z Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2879 File: sql/dijkstra/dijkstra.sql:56-57 Timestamp: 2025-04-29T18:59:06.824Z Learning: The maintainer prefers not to add inline comments to internal function calls like _pgr_dijkstra, even when they contain multiple boolean and numeric flags.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/trsp/trspVia_withPoints_driver.cpp:154-154 Timestamp: 2025-01-19T16:19:16.974Z Learning: Error messages in pgRouting should be kept minimal and consistent to maintain compatibility with pgtap tests.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2723 File: tools/developer/run.sh:192-194 Timestamp: 2025-01-19T13:16:44.066Z Learning: For the pgRouting repository, keep review comments concise and avoid excessive detail in suggestions.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2868 File: tools/testers/contractionHierarchies_data.sql:73-85 Timestamp: 2025-04-24T23:29:43.790Z Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/coloring/edgeColoring_driver.cpp:87-87 Timestamp: 2025-01-19T16:17:53.496Z Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/trsp/trsp_driver.cpp:231-232 Timestamp: 2025-01-19T16:18:09.359Z Learning: In pgRouting, modifications to error message handling need careful consideration as they impact pgtap test cases which verify exact error message content.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/dijkstra/dijkstra_driver.cpp:147-148 Timestamp: 2025-01-19T16:18:55.529Z Learning: In pgRouting, error messages should not be modified without careful consideration as they are verified by pgtap tests. Changes to error messages require corresponding updates to test cases.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/trsp/trsp_withPoints_driver.cpp:120-121 Timestamp: 2025-01-19T16:18:00.783Z Learning: In pgRouting, modifying error messages requires updating pgtap test expectations, so such changes should be carefully considered due to the test maintenance burden.doc/src/release_notes.rst (1)
49-52: LGTM!The new "Code enhancement" subsection follows the established formatting pattern in the release notes, with appropriate RST markup and proper issue reference.
doc/src/pgRouting-introduction.rst (1)
66-66: LGTM!Using the
|release|substitution variable instead of a hardcoded version string improves maintainability and ensures the contributor section heading automatically reflects the current release version.locale/en/LC_MESSAGES/pgrouting_doc_strings.po (3)
11-11: POT-Creation-Date update is consistent with regenerated docsThe timestamp bump reflects a regenerated message catalog and has no behavioral impact. No action needed.
8326-8331: Contributors strings and release placeholder look consistentSwitching to “|release|” in the “Individuals in this release …” string and updating the contributor list to include “Mohit Rawat” while keeping ordering/wording consistent with the English docs is appropriate. Given this file is generated from the .pot/docs, these diffs look correct.
Also applies to: 8397-8401
16167-16174: New “Code enhancement” entry correctly records ordering standardizationThe added “Code enhancement” msgid and the note for issue
#3025(“Ordering functions standardized”) align with the PR’s purpose and release notes. This is the expected .po counterpart to the NEWS/doc change.locale/pot/pgrouting_doc_strings.pot (3)
11-11: POT-Creation-Date update is fineThe timestamp change reflects a regenerated POT and doesn’t affect behavior; no action needed.
Based on learnings, auto-generated updates to this .pot file are expected and acceptable.
7147-7151: Release contributors msgids look correctSwitching to the
|release|placeholder and updating the contributor list keeps the string reusable across versions and preserves alphabetical ordering by first name; no issues.
13510-13514: New release-notes entries for issue #3025 are consistentThe added “Code enhancement” header and the
#3025: Ordering functions standardizedmsgid align with the PR’s objective to standardize ordering functions and are correctly structured for localization.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ordering/ordering_driver.cpp (1)
153-155: Fix inverted ternary logic in exception handler.The ternary condition appears inverted. When
hint.empty()is true, it callsto_pg_msg(hint)which would return an empty message. The intended logic is likely to uselogwhenhintis empty.🔎 Proposed fix
} catch (const std::string &ex) { *err_msg = to_pg_msg(ex); - *log_msg = hint.empty()? to_pg_msg(hint) : to_pg_msg(log); + *log_msg = hint.empty() ? to_pg_msg(log) : to_pg_msg(hint); }
🤖 Fix all issues with AI agents
In @include/c_common/enums.h:
- Line 2: The file header comment mistakenly says "File: ordering_process.h"
while the actual header is enums.h; update the top-of-file comment in
include/c_common/enums.h (the file-level comment string) to read "File: enums.h"
(or otherwise reflect the correct filename) so the header comment matches the
actual file name.
- Around line 28-33: Add an explanatory comment above the TOPOSORT enum member
in enum Which to document the intentional gap between KING and TOPOSORT (that
values 3–10 are reserved for additional undirected orderings and 11+ are for
directed orderings); update the comment near TOPOSORT to reference KING and the
reserved range so future maintainers understand the numbering scheme.
In @src/ordering/ordering_driver.cpp:
- Around line 62-72: Change do_ordering's edges_sql parameter to a const
reference to avoid copying: update the function signature in do_ordering to take
const std::string& edges_sql and update the corresponding declaration in
include/drivers/ordering_driver.hpp to match; ensure all call sites still pass
std::string or const std::string& (no other code changes needed).
In @src/ordering/ordering_process.cpp:
- Around line 80-82: The TOPOSORT switch case logs the wrong function name;
update the time_msg call inside the TOPOSORT case (where TOPOSORT is handled and
time_msg(std::string("processing pgr_kingOrdering").c_str(), start_t, clock())
is used) to log "processing pgr_topologicalSort" instead of "processing
pgr_kingOrdering" so the debug output correctly reflects the pgr_topologicalSort
operation.
- Around line 70-83: The switch on the enum Which (variable which) lacks a
default branch; add a default case that handles unexpected enum values by
calling time_msg (or a logger) with a clear message including the numeric value
of which (and maybe the start_t/clock() timing) to aid debugging, e.g., format
"processing unknown ordering: %d" and preserve existing behavior for known cases
SLOAN, CUTCHILL, KING, TOPOSORT; this makes the code defensive against future
enum additions or invalid values.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
include/c_common/enums.hinclude/cpp_common/to_postgres.hppinclude/drivers/ordering_driver.hppinclude/process/ordering_process.hlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.potsrc/ordering/cuthillMckeeOrdering.csrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/kingOrdering.csrc/ordering/ordering_driver.cppsrc/ordering/ordering_process.cppsrc/ordering/sloanOrdering.csrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.c
🧰 Additional context used
🧠 Learnings (40)
📓 Common learnings
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2871
File: include/drivers/transitiveClosure/transitiveClosure_driver.h:45-49
Timestamp: 2025-04-25T02:12:17.271Z
Learning: The pgRouting project avoids using named parameters in C header files to prevent conflicts with Doxygen documentation.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_withPoints_driver.cpp:154-154
Timestamp: 2025-01-19T16:19:16.974Z
Learning: Error messages in pgRouting should be kept minimal and consistent to maintain compatibility with pgtap tests.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:130-144
Timestamp: 2025-01-19T13:16:52.140Z
Learning: In the pgRouting project, function documentation should be kept minimal and concise, focusing on essential information only.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2874
File: doc/utilities/pgr_separateCrossing.rst:26-27
Timestamp: 2025-04-25T15:12:03.608Z
Learning: In pgRouting documentation, official functions don't include version information in their index entries (`:single:` directives). Only deprecated functions include when they were deprecated in the index entries.
📚 Learning: 2026-01-06T20:27:35.905Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3026
File: src/ordering/sloanOrdering.cpp:46-89
Timestamp: 2026-01-06T20:27:35.905Z
Learning: In C++ code, prefer direct Boost Graph Library calls over wrapper methods when accessing graph properties to future-proof against planned removal of wrapper base classes. Validate by replacing wrappers like graph.num_vertices() with direct BGL calls such as boost::num_vertices(graph.graph) (adjusting to the actual member names in your code). This guidance applies across C++ sources in the repository where similar graph wrappers are used.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/ordering_process.cpp
📚 Learning: 2025-01-19T16:18:49.447Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/bellman_ford/bellman_ford_neg_driver.cpp:119-119
Timestamp: 2025-01-19T16:18:49.447Z
Learning: The edge count check in bellman_ford_neg_driver.cpp is part of code that is not yet in use, as of January 2025.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-09-14T17:29:04.360Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: CMakeLists.txt:96-104
Timestamp: 2025-09-14T17:29:04.360Z
Learning: In pgRouting, BOOST_THREAD_LIBRARIES is referenced in CMakeLists.txt but the project doesn't actually use Boost threading functionality in the source code, so the LINK_LIBRARIES(${BOOST_THREAD_LIBRARIES}) line should be removed as unused legacy code.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-01-19T13:16:44.066Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:192-194
Timestamp: 2025-01-19T13:16:44.066Z
Learning: For the pgRouting repository, keep review comments concise and avoid excessive detail in suggestions.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.c
📚 Learning: 2025-09-14T17:29:04.360Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: CMakeLists.txt:96-104
Timestamp: 2025-09-14T17:29:04.360Z
Learning: In pgRouting CMakeLists.txt, the project does not use the Boost thread component, so LINK_LIBRARIES(${BOOST_THREAD_LIBRARIES}) should be removed rather than adding COMPONENTS thread to find_package(Boost).
Applied to files:
src/ordering/cuthillMckeeOrdering.cpp
📚 Learning: 2025-02-27T23:05:28.589Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: sql/common/_checkquery.sql:33-33
Timestamp: 2025-02-27T23:05:28.589Z
Learning: When reviewing PostgreSQL/pgRouting code, respect the maintainer's preference for brevity over additional comments when variable names are descriptive.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.clocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-09-15T00:39:05.272Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: NEWS.md:16-24
Timestamp: 2025-09-15T00:39:05.272Z
Learning: In pgRouting, editorial improvements to NEWS.md content should be implemented via the notes2news.pl script or its source files, since NEWS.md is auto-generated and manual edits would be overwritten.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-27T01:49:25.212Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.clocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T13:17:02.243Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:62-68
Timestamp: 2025-01-19T13:17:02.243Z
Learning: For pgRouting project, CMake configuration comments should be kept minimal and concise, avoiding extensive explanations of build options and configurations.
Applied to files:
src/ordering/cuthillMckeeOrdering.cpp
📚 Learning: 2025-04-24T23:36:04.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: include/drivers/contraction/contractionHierarchies_driver.h:1-73
Timestamp: 2025-04-24T23:36:04.580Z
Learning: The copyright year for pgRouting should be set to 2025 in new files, not 2024, as confirmed by the project maintainer.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-02-06T20:50:00.577Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.c
📚 Learning: 2025-12-30T19:45:32.986Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3004
File: .clang-tidy:9-11
Timestamp: 2025-12-30T19:45:32.986Z
Learning: In pgRouting's .clang-tidy file (and similar list configurations), the last line of lists should not have a trailing comma, per project convention.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-04-24T23:29:43.790Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.cinclude/c_common/enums.h
📚 Learning: 2025-04-25T00:59:52.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.c
📚 Learning: 2025-01-19T16:18:00.783Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_withPoints_driver.cpp:120-121
Timestamp: 2025-01-19T16:18:00.783Z
Learning: In pgRouting, modifying error messages requires updating pgtap test expectations, so such changes should be carefully considered due to the test maintenance burden.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/ordering_process.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:19:16.974Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_withPoints_driver.cpp:154-154
Timestamp: 2025-01-19T16:19:16.974Z
Learning: Error messages in pgRouting should be kept minimal and consistent to maintain compatibility with pgtap tests.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.clocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:18:09.359Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_driver.cpp:231-232
Timestamp: 2025-01-19T16:18:09.359Z
Learning: In pgRouting, modifications to error message handling need careful consideration as they impact pgtap test cases which verify exact error message content.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/topologicalSort.csrc/ordering/ordering_process.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:17:53.496Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/coloring/edgeColoring_driver.cpp:87-87
Timestamp: 2025-01-19T16:17:53.496Z
Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cppsrc/ordering/ordering_process.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-09-17T02:09:53.993Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: doc/contraction/pgr_contraction.rst:0-0
Timestamp: 2025-09-17T02:09:53.993Z
Learning: In pgRouting documentation, pseudocode blocks can use generic variable names that don't necessarily match the exact parameter names in the API, as decided by maintainer cvvergara for doc/contraction/pgr_contraction.rst.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/topologicalSort.c
📚 Learning: 2025-01-19T16:17:38.435Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/pickDeliver/pickDeliver_driver.cpp:69-69
Timestamp: 2025-01-19T16:17:38.435Z
Learning: Error messages in pgRouting are part of pgtap test assertions and should not be modified without updating the corresponding tests.
Applied to files:
src/ordering/ordering_driver.cppsrc/ordering/sloanOrdering.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-06T20:50:07.967Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Applied to files:
src/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/ordering_process.cppinclude/cpp_common/to_postgres.hpplocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T16:33:56.959Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: sql/sigs/pgrouting--3.8.sig:125-136
Timestamp: 2025-02-27T16:33:56.959Z
Learning: The file sql/sigs/pgrouting--*.sig is automatically generated by a command and cannot be manually modified. Comments about naming conventions or other improvements should be directed at the source code that generates these signatures, not at the signature file itself.
Applied to files:
src/ordering/sloanOrdering.cpp
📚 Learning: 2025-02-27T16:34:57.518Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: sql/sigs/pgrouting--3.8.sig:274-275
Timestamp: 2025-02-27T16:34:57.518Z
Learning: The file `sql/sigs/pgrouting--3.8.sig` is auto-generated by a command and should not be modified manually.
Applied to files:
src/ordering/sloanOrdering.cppinclude/cpp_common/to_postgres.hpp
📚 Learning: 2025-04-29T18:59:06.824Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2879
File: sql/dijkstra/dijkstra.sql:56-57
Timestamp: 2025-04-29T18:59:06.824Z
Learning: The maintainer prefers not to add inline comments to internal function calls like _pgr_dijkstra, even when they contain multiple boolean and numeric flags.
Applied to files:
src/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.c
📚 Learning: 2025-12-31T06:01:45.144Z
Learnt from: Mohit242-bit
Repo: pgRouting/pgrouting PR: 3018
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:11-11
Timestamp: 2025-12-31T06:01:45.144Z
Learning: In pgRouting/pgrouting, CI/Weblate may update locale/pot/pgrouting_doc_strings.pot and locale/en/LC_MESSAGES/pgrouting_doc_strings.po in PRs after .rst changes. These auto-generated updates are acceptable and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
src/ordering/sloanOrdering.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-10-30T16:43:11.554Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2989
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:10-10
Timestamp: 2025-10-30T16:43:11.554Z
Learning: When a locale translation change (e.g., locale/es/LC_MESSAGES/pgrouting_doc_strings.po) is introduced by a Weblate-authored commit within a PR, it is compliant with pgRouting’s workflow and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
src/ordering/sloanOrdering.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T23:09:12.162Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Applied to files:
src/ordering/sloanOrdering.cpplocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:18:55.529Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/dijkstra/dijkstra_driver.cpp:147-148
Timestamp: 2025-01-19T16:18:55.529Z
Learning: In pgRouting, error messages should not be modified without careful consideration as they are verified by pgtap tests. Changes to error messages require corresponding updates to test cases.
Applied to files:
src/ordering/topologicalSort.csrc/ordering/ordering_process.cpplocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/cuthillMckeeOrdering.clocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2026-01-06T20:27:35.905Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3026
File: src/ordering/sloanOrdering.cpp:46-89
Timestamp: 2026-01-06T20:27:35.905Z
Learning: In pgRouting, the team plans to eventually stop using the base graph class wrappers. Direct Boost Graph Library calls (e.g., `boost::num_vertices(graph.graph)`) are acceptable and preferred over wrapper methods (e.g., `graph.num_vertices()`) as this future-proofs the code against the planned removal of the base class.
Applied to files:
include/cpp_common/to_postgres.hpp
📚 Learning: 2025-02-27T23:08:14.497Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_degree.rst:28-35
Timestamp: 2025-02-27T23:08:14.497Z
Learning: In the pgRouting documentation, historical version notes should be retained to show the evolution of functions across different versions.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-24T23:34:23.099Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: NEWS.md:46-47
Timestamp: 2025-04-24T23:34:23.099Z
Learning: For pgRouting, NEWS.md should be updated using the generator script (tools/release-scripts/notes2news.pl) rather than edited directly, which is the correct procedure that the project maintainers follow.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-22T22:41:34.709Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:47-50
Timestamp: 2025-04-22T22:41:34.709Z
Learning: In pgRouting, NEWS.md is a generated file created by the tools/release-scripts/notes2news.pl script and should not be modified manually.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:41:25.296Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:56-57
Timestamp: 2025-04-22T22:41:25.296Z
Learning: In pgRouting, NEWS.md is an automatically generated file that should not be modified manually. Changes to the NEWS.md content should be made by modifying the generator script (tools/release-scripts/notes2news.pl) or its source files.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-09-14T16:43:16.715Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/boost_version.yml:134-135
Timestamp: 2025-09-14T16:43:16.715Z
Learning: In pgrouting, the installed PostgreSQL extension version is normalized to major.minor.patch format (e.g., "4.0.0") regardless of any pre-release suffixes like -alpha1, -beta, or -rc that are used in the source/build versioning. Database setup scripts should use the normalized version format.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:31.895Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/contraction/contractGraph_driver.cpp:187-187
Timestamp: 2025-01-19T16:17:31.895Z
Learning: Changes to error messages in pgRouting must carefully consider impacts on pgtap tests, as these tests verify exact message content.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-01-19T16:17:42.135Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_driver.cpp:151-151
Timestamp: 2025-01-19T16:17:42.135Z
Learning: Modifying error messages in pgRouting requires updating the corresponding pgtap tests to match the new messages, as they verify exact string outputs.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-04-22T22:59:10.377Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:15446-15454
Timestamp: 2025-04-22T22:59:10.377Z
Learning: En pgRouting, las traducciones al español de las cadenas de documentación (por ejemplo, en `pgrouting_doc_strings.po`) se gestionan a través de Weblate y no se deben sugerir ni modificar directamente en los pull requests.
Applied to files:
locale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T16:32:00.573Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:0-0
Timestamp: 2025-02-27T16:32:00.573Z
Learning: Modifications to locale files in the pgRouting project should be handled through an action commit or through the Weblate translation platform, not via direct edits.
Applied to files:
locale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T16:31:49.972Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:2740-2740
Timestamp: 2025-02-27T16:31:49.972Z
Learning: Modifications to locale files in pgRouting (like .po files) are done through action commits or in weblate, not through direct manual edits in PRs.
Applied to files:
locale/pot/pgrouting_doc_strings.pot
🧬 Code graph analysis (6)
include/process/ordering_process.h (1)
src/ordering/ordering_process.cpp (2)
pgr_process_ordering(50-94)pgr_process_ordering(50-54)
src/ordering/cuthillMckeeOrdering.cpp (3)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)src/ordering/sloanOrdering.cpp (2)
colors(60-60)inv_permutation(57-57)
src/ordering/ordering_driver.cpp (5)
include/cpp_common/to_postgres.hpp (2)
get_vertexId(84-97)get_vertexId(84-88)src/ordering/sloanOrdering.cpp (2)
sloanOrdering(46-89)sloanOrdering(47-47)include/ordering/sloanOrdering.hpp (1)
sloanOrdering(41-41)include/ordering/kingOrdering.hpp (1)
kingOrdering(42-42)src/ordering/cuthillMckeeOrdering.cpp (2)
cuthillMckeeOrdering(59-97)cuthillMckeeOrdering(60-60)
src/ordering/ordering_process.cpp (1)
src/common/time_msg.c (1)
time_msg(31-39)
include/cpp_common/to_postgres.hpp (2)
include/coloring/sequentialVertexColoring.hpp (2)
graph(74-103)graph(74-74)include/traversal/depthFirstSearch.hpp (4)
graph(72-94)graph(72-76)graph(115-156)graph(115-120)
src/ordering/cuthillMckeeOrdering.c (1)
src/ordering/ordering_process.cpp (2)
pgr_process_ordering(50-94)pgr_process_ordering(50-54)
🪛 Cppcheck (2.19.0)
src/ordering/cuthillMckeeOrdering.cpp
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 80-80: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 60-60: The function 'cuthillMckeeOrdering' is never used.
(unusedFunction)
src/ordering/ordering_driver.cpp
[information] 40-40: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 45-45: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 80-80: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[performance] 64-64: Function parameter 'edges_sql' should be passed by const reference.
(passedByValue)
[style] 57-57: The function 'set_ids' is never used.
(unusedFunction)
[style] 71-71: The function 'start_id' is never used.
(unusedFunction)
[style] 73-73: The function 'end_id' is never used.
(unusedFunction)
[style] 75-75: The function 'tot_cost' is never used.
(unusedFunction)
[style] 59-59: The function 'dest_id' is never used.
(unusedFunction)
[style] 63-63: The function 'do_ordering' is never used.
(unusedFunction)
[performance] 113-113: Function parameter 'results' should be passed by const reference.
(passedByValue)
[style] 111-111: The function 'vector_to_tuple' is never used.
(unusedFunction)
src/ordering/sloanOrdering.cpp
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 80-80: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'sloanOrdering' is never used.
(unusedFunction)
src/ordering/topologicalSort.c
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 43-43: The function '_pgr_topologicalsort' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
src/ordering/cuthillMckeeOrdering.c
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 42-42: The function '_pgr_cuthillmckeeordering' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
⏰ 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). (20)
- GitHub Check: Ubuntu clang (Debug, ubuntu-22.04)
- GitHub Check: Build (56, 3.1.2)
- GitHub Check: Build (56, 3.0.6)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Build (56, 3.1.3)
- GitHub Check: Build (56, 3.1.0)
- GitHub Check: Build (56, 3.7.0)
- GitHub Check: Build (56, 3.6.3)
- GitHub Check: Build (56, 3.4.1)
- GitHub Check: Ubuntu clang (Release, ubuntu-latest)
- GitHub Check: Ubuntu clang (Debug, ubuntu-latest)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Build (56, 3.7.1)
- GitHub Check: Build (56, 3.5.0)
- GitHub Check: Build (56, 4.0.0)
- GitHub Check: macos (macos-latest)
- GitHub Check: Build (56, 3.7.2)
- GitHub Check: Build (56, 3.4.0)
- GitHub Check: Build (56, 3.0.2)
- GitHub Check: macos (macos-14)
🔇 Additional comments (16)
locale/en/LC_MESSAGES/pgrouting_doc_strings.po (2)
8326-8327: Use of|release|token for contributor header looks goodSwitching from a hard-coded version string to
|release|keeps the string reusable across releases and consistent with the rest of the docs.
16167-16173: New “Code enhancement” entry for issue #3025 is consistentThe added heading and linked description correctly document the ordering-functions standardization and follow existing release-notes formatting.
locale/pot/pgrouting_doc_strings.pot (2)
7147-7147: Use of|release|placeholder in contributor string looks correctSwitching from a hard-coded version to
|release|keeps this msgid stable across releases and matches the rest of the docs’ substitution pattern. No issues here.
13510-13515: New “Code enhancement” entry for issue #3025 is consistentThe added “Code enhancement” label and the
#3025description cleanly expose the ordering-standardization work in public strings and align with the PR objectives. Nothing to adjust.src/ordering/sloanOrdering.c (1)
57-57: LGTM! Improved code clarity with named constant.Replacing the magic number
0with the symbolic constantSLOANenhances readability and aligns with the type-safe enum-based refactor.src/ordering/kingOrdering.c (1)
57-57: LGTM! Improved code clarity with named constant.Replacing the magic number
2with the symbolic constantKINGenhances readability and aligns with the type-safe enum-based refactor.include/drivers/ordering_driver.hpp (1)
42-47: LGTM! Type-safe API with enum parameter.The addition of
enums.hand the change frominttoWhichin the function signature provides compile-time type safety and prevents invalid ordering type values.include/process/ordering_process.h (2)
41-41: LGTM! Necessary include for enum Which.The include enables the use of the
Whichenum type in the function signature.
46-46: LGTM! Type-safe API with enum parameter.The change from
inttoenum Whichprovides compile-time type safety and is consistent with the implementation inordering_process.cpp.include/c_common/enums.h (1)
28-33: Note: Copy-paste error in ordering_process.cpp.While reviewing the related code in
src/ordering/ordering_process.cpp(line 80), theTOPOSORTcase has a copy-paste error in the time message: it says "processing pgr_kingOrdering" but should say "processing pgr_topologicalSort".Based on the relevant code snippet from
src/ordering/ordering_process.cpp:case TOPOSORT: time_msg(std::string("processing pgr_kingOrdering").c_str(), start_t, clock()); break;Should be:
case TOPOSORT: time_msg(std::string("processing pgr_topologicalSort").c_str(), start_t, clock()); break;⛔ Skipped due to learnings
Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/dijkstra/dijkstra_driver.cpp:147-148 Timestamp: 2025-01-19T16:18:55.529Z Learning: In pgRouting, error messages should not be modified without careful consideration as they are verified by pgtap tests. Changes to error messages require corresponding updates to test cases.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/trsp/trspVia_driver.cpp:151-151 Timestamp: 2025-01-19T16:17:42.135Z Learning: Modifying error messages in pgRouting requires updating the corresponding pgtap tests to match the new messages, as they verify exact string outputs.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/trsp/trsp_driver.cpp:231-232 Timestamp: 2025-01-19T16:18:09.359Z Learning: In pgRouting, modifications to error message handling need careful consideration as they impact pgtap test cases which verify exact error message content.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/trsp/trsp_withPoints_driver.cpp:120-121 Timestamp: 2025-01-19T16:18:00.783Z Learning: In pgRouting, modifying error messages requires updating pgtap test expectations, so such changes should be carefully considered due to the test maintenance burden.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/contraction/contractGraph_driver.cpp:187-187 Timestamp: 2025-01-19T16:17:31.895Z Learning: Changes to error messages in pgRouting must carefully consider impacts on pgtap tests, as these tests verify exact message content.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/planar/boyerMyrvold_driver.cpp:100-100 Timestamp: 2025-01-19T16:17:46.713Z Learning: Changes to error message handling in pgRouting need to consider impact on pgtap tests, as they may have assertions that depend on specific message behavior.Learnt from: cvvergara Repo: pgRouting/pgrouting PR: 2725 File: src/coloring/edgeColoring_driver.cpp:87-87 Timestamp: 2025-01-19T16:17:53.496Z Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.include/cpp_common/to_postgres.hpp (1)
74-97: LGTM!The new
get_vertexIdfunction is well-implemented:
- Correctly passes data by const reference
- Uses clear variable naming (
vfor vertex)- Properly allocates and populates result tuples
- Direct BGL access via
graph.graph[v].idaligns with project direction per learningssrc/ordering/cuthillMckeeOrdering.c (1)
41-103: LGTM!The refactoring correctly integrates with the new unified ordering interface:
- Uses
CUTCHILLenum constant instead of magic number- Type change to
int64_t*aligns with the newpgr_process_orderingsignature- Column count reduction to 2 matches the simplified output format
- SRF handling is correct
src/ordering/cuthillMckeeOrdering.cpp (1)
59-97: LGTM!The implementation is well-structured and follows project conventions:
- Uses direct BGL calls (
boost::num_vertices,boost::get, etc.) as preferred per learnings- Proper interrupt check before computation
- Exception handling pattern matches other ordering implementations
- Header include added as previously requested
src/ordering/sloanOrdering.cpp (1)
46-89: LGTM!The implementation correctly follows project conventions:
- Direct BGL calls (
boost::num_vertices(graph.graph)) as preferred per learnings- Proper property map setup for Sloan ordering (color, degree, priority maps)
- Interrupt check before expensive computation
- Consistent exception handling pattern
src/ordering/topologicalSort.c (1)
42-102: LGTM!The refactoring correctly integrates with the unified ordering interface:
- Uses
TOPOSORTenum constant as previously requested- Type changes to
int64_t*align with new API- SRF handling is properly updated
src/ordering/ordering_driver.cpp (1)
51-60: LGTM on the generic process template.The template function cleanly encapsulates the common pattern for all ordering algorithms: insert edges, run ordering, and convert results to Postgres format. This is a good abstraction.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @include/c_common/enums.h:
- Around line 28-33: Add an explicit comment in the enum Which explaining that
the numeric gap between KING and TOPOSORT is intentional and reserving values
3–10 for future order types or subranges (e.g., undirected orderings in 0–10,
directed in 11+). Update the enum declaration near symbols Which, SLOAN, KING,
and TOPOSORT to document the reserved range and the rationale so maintainers
understand the numbering scheme.
In @src/ordering/ordering_driver.cpp:
- Line 115: The statement that constructs digraph has a redundant double
semicolon: fix the declaration of variable digraph in the function/context where
"DirectedGraph digraph = which >= 11? DirectedGraph(vertices) :
DirectedGraph();;" by removing the extra trailing semicolon so it reads with a
single semicolon; ensure the code uses the existing symbols DirectedGraph,
digraph and the conditional expression involving which and vertices unchanged
aside from the removed semicolon.
In @src/ordering/ordering_process.cpp:
- Line 81: The timing message for the TOPOSORT case mistakenly logs "processing
pgr_pgr_topologicalSort"; update the string passed to time_msg in the TOPOSORT
handling so it reads "processing pgr_topologicalSort" to match the function name
and remove the duplicate "pgr_" prefix; locate the time_msg call that currently
constructs std::string("processing pgr_pgr_topologicalSort").c_str() and change
the literal accordingly.
- Around line 72-78: The timing message for pgr_sloanOrdering has an extra
leading space causing inconsistent log formatting; update the string passed to
time_msg in the branch handling pgr_sloanOrdering (the call in the case that
precedes CUTCHILL and KING) to match the others by removing the leading space so
it reads "processing pgr_sloanOrdering" (referencing the time_msg calls for
pgr_cuthillMckeeOrdering and pgr_kingOrdering to ensure identical
spacing/format).
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
include/c_common/enums.hsrc/ordering/ordering_driver.cppsrc/ordering/ordering_process.cpp
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3026
File: src/ordering/sloanOrdering.cpp:46-89
Timestamp: 2026-01-06T20:27:35.905Z
Learning: In pgRouting, the team plans to eventually stop using the base graph class wrappers. Direct Boost Graph Library calls (e.g., `boost::num_vertices(graph.graph)`) are acceptable and preferred over wrapper methods (e.g., `graph.num_vertices()`) as this future-proofs the code against the planned removal of the base class.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2874
File: doc/utilities/pgr_separateCrossing.rst:26-27
Timestamp: 2025-04-25T15:12:03.608Z
Learning: In pgRouting documentation, official functions don't include version information in their index entries (`:single:` directives). Only deprecated functions include when they were deprecated in the index entries.
📚 Learning: 2025-02-06T20:50:07.967Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Applied to files:
src/ordering/ordering_process.cpp
📚 Learning: 2025-01-19T16:18:09.359Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_driver.cpp:231-232
Timestamp: 2025-01-19T16:18:09.359Z
Learning: In pgRouting, modifications to error message handling need careful consideration as they impact pgtap test cases which verify exact error message content.
Applied to files:
src/ordering/ordering_process.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:17:53.496Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/coloring/edgeColoring_driver.cpp:87-87
Timestamp: 2025-01-19T16:17:53.496Z
Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.
Applied to files:
src/ordering/ordering_process.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:18:55.529Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/dijkstra/dijkstra_driver.cpp:147-148
Timestamp: 2025-01-19T16:18:55.529Z
Learning: In pgRouting, error messages should not be modified without careful consideration as they are verified by pgtap tests. Changes to error messages require corresponding updates to test cases.
Applied to files:
src/ordering/ordering_process.cpp
📚 Learning: 2025-02-06T20:50:00.577Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Applied to files:
src/ordering/ordering_process.cpp
📚 Learning: 2025-01-19T16:18:00.783Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_withPoints_driver.cpp:120-121
Timestamp: 2025-01-19T16:18:00.783Z
Learning: In pgRouting, modifying error messages requires updating pgtap test expectations, so such changes should be carefully considered due to the test maintenance burden.
Applied to files:
src/ordering/ordering_process.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-02-27T23:09:12.162Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Applied to files:
src/ordering/ordering_process.cpp
📚 Learning: 2025-01-27T01:49:25.212Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Applied to files:
src/ordering/ordering_process.cpp
📚 Learning: 2026-01-06T20:27:35.905Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3026
File: src/ordering/sloanOrdering.cpp:46-89
Timestamp: 2026-01-06T20:27:35.905Z
Learning: In C++ code, prefer direct Boost Graph Library calls over wrapper methods when accessing graph properties to future-proof against planned removal of wrapper base classes. Validate by replacing wrappers like graph.num_vertices() with direct BGL calls such as boost::num_vertices(graph.graph) (adjusting to the actual member names in your code). This guidance applies across C++ sources in the repository where similar graph wrappers are used.
Applied to files:
src/ordering/ordering_process.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-04-29T18:59:06.824Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2879
File: sql/dijkstra/dijkstra.sql:56-57
Timestamp: 2025-04-29T18:59:06.824Z
Learning: The maintainer prefers not to add inline comments to internal function calls like _pgr_dijkstra, even when they contain multiple boolean and numeric flags.
Applied to files:
include/c_common/enums.h
📚 Learning: 2025-02-27T23:05:28.589Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: sql/common/_checkquery.sql:33-33
Timestamp: 2025-02-27T23:05:28.589Z
Learning: When reviewing PostgreSQL/pgRouting code, respect the maintainer's preference for brevity over additional comments when variable names are descriptive.
Applied to files:
include/c_common/enums.hsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:18:49.447Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/bellman_ford/bellman_ford_neg_driver.cpp:119-119
Timestamp: 2025-01-19T16:18:49.447Z
Learning: The edge count check in bellman_ford_neg_driver.cpp is part of code that is not yet in use, as of January 2025.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-12-30T19:45:32.986Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3004
File: .clang-tidy:9-11
Timestamp: 2025-12-30T19:45:32.986Z
Learning: In pgRouting's .clang-tidy file (and similar list configurations), the last line of lists should not have a trailing comma, per project convention.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-04-24T23:29:43.790Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-04-25T00:59:52.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T13:16:44.066Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:192-194
Timestamp: 2025-01-19T13:16:44.066Z
Learning: For the pgRouting repository, keep review comments concise and avoid excessive detail in suggestions.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:19:16.974Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_withPoints_driver.cpp:154-154
Timestamp: 2025-01-19T16:19:16.974Z
Learning: Error messages in pgRouting should be kept minimal and consistent to maintain compatibility with pgtap tests.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-09-17T02:09:53.993Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: doc/contraction/pgr_contraction.rst:0-0
Timestamp: 2025-09-17T02:09:53.993Z
Learning: In pgRouting documentation, pseudocode blocks can use generic variable names that don't necessarily match the exact parameter names in the API, as decided by maintainer cvvergara for doc/contraction/pgr_contraction.rst.
Applied to files:
src/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:17:38.435Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/pickDeliver/pickDeliver_driver.cpp:69-69
Timestamp: 2025-01-19T16:17:38.435Z
Learning: Error messages in pgRouting are part of pgtap test assertions and should not be modified without updating the corresponding tests.
Applied to files:
src/ordering/ordering_driver.cpp
🧬 Code graph analysis (2)
src/ordering/ordering_process.cpp (1)
src/common/time_msg.c (1)
time_msg(31-39)
src/ordering/ordering_driver.cpp (5)
include/cpp_common/to_postgres.hpp (2)
get_vertexId(84-97)get_vertexId(84-88)src/ordering/sloanOrdering.cpp (2)
sloanOrdering(46-89)sloanOrdering(47-47)include/ordering/sloanOrdering.hpp (1)
sloanOrdering(41-41)include/ordering/kingOrdering.hpp (1)
kingOrdering(42-42)src/ordering/cuthillMckeeOrdering.cpp (2)
cuthillMckeeOrdering(59-97)cuthillMckeeOrdering(60-60)
🪛 Cppcheck (2.19.0)
src/ordering/ordering_driver.cpp
[information] 40-40: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 45-45: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 81-81: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[style] 57-57: The function 'set_ids' is never used.
(unusedFunction)
[style] 71-71: The function 'start_id' is never used.
(unusedFunction)
[style] 73-73: The function 'end_id' is never used.
(unusedFunction)
[style] 75-75: The function 'tot_cost' is never used.
(unusedFunction)
[style] 59-59: The function 'dest_id' is never used.
(unusedFunction)
[style] 63-63: The function 'do_ordering' is never used.
(unusedFunction)
[performance] 113-113: Function parameter 'results' should be passed by const reference.
(passedByValue)
[style] 111-111: The function 'vector_to_tuple' is never used.
(unusedFunction)
[style] 145-145: The function 'matrix_to_tuple' is never used.
(unusedFunction)
⏰ 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). (21)
- GitHub Check: macos (macos-latest)
- GitHub Check: macos (macos-14)
- GitHub Check: Build (56, 3.3.5)
- GitHub Check: Build (56, 3.8.0)
- GitHub Check: Build (56, 3.3.3)
- GitHub Check: Build (56, 3.7.1)
- GitHub Check: Ubuntu clang (Debug, ubuntu-22.04)
- GitHub Check: Ubuntu clang (Debug, ubuntu-latest)
- GitHub Check: Ubuntu clang (Release, ubuntu-latest)
- GitHub Check: Ubuntu clang (Release, ubuntu-22.04)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build
🔇 Additional comments (4)
src/ordering/ordering_process.cpp (1)
83-84: Default case correctly handles unexpected enum values.The default case appropriately allows the function to continue and return "No results found" for unexpected
whichvalues, consistent with the project's design philosophy.src/ordering/ordering_driver.cpp (3)
64-64: Parameter now passed by const reference.The
edges_sqlparameter is now correctly passed by const reference, improving efficiency by avoiding unnecessary string copies.
51-60: Well-designed template function for unified processing.The
processtemplate effectively consolidates edge insertion, ordering execution, and result conversion into a single reusable pathway. Usingget_vertexIdprovides a clean abstraction for result mapping.
118-137: Switch dispatch correctly implements multi-algorithm routing.The switch statement cleanly routes each ordering algorithm to the appropriate graph type and processing function. The default case appropriately allows graceful handling of unexpected values per the project's design.
- Works only for undirected graph - Breaking into header and implementation
- Works only for undirected graph - Breaking into header and implementation
- Works only for undirected graph - Breaking into header and implementation
- Works only for directed graph - Breaking into header and implementation
- Insted of if cascade: using switch - Using the to_postgress::get_vertedId - Including CutchillMckee ordering - General Cleanup
cvvergara
force-pushed
the
standardize-ordering-functions-code
branch
from
35a006f to
c4ab9e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/ordering/ordering_process.cpp (1)
43-49: Update comment to reflect enum-based API.The comment still references numeric values (
which = 0,which = 1, etc.) but the code now usesenum Whichwith named constants (SLOAN, CUTCHILL, KING, TOPOSORT).📝 Proposed fix
/** - which = 0 -> sloan - which = 1 -> cuthillmckee - which = 2 -> king + which = SLOAN -> sloan + which = CUTCHILL -> cuthillmckee + which = KING -> king + which = TOPOSORT -> topological sort This is c++ code, linked as C code, because pgr_process_foo is called from C code */src/ordering/ordering_driver.cpp (1)
157-157: Fix inverted ternary logic.The condition is backwards: when
hint.empty()is true, it assignshint(which is empty); when false, it assignslog. This should be the opposite.🐛 Proposed fix
- *log_msg = hint.empty()? to_pg_msg(hint) : to_pg_msg(log); + *log_msg = hint.empty()? to_pg_msg(log) : to_pg_msg(hint);
🤖 Fix all issues with AI agents
In @include/ordering/cuthillMckeeOrdering.hpp:
- Around line 41-42: Change the three ordering functions to accept the graph by
const reference: update the declarations and definitions of
cuthillMckeeOrdering, kingOrdering, and sloanOrdering to use const
pgrouting::UndirectedGraph &graph instead of a non-const reference, and adjust
any internal calls if necessary to use const-compatible Boost algorithms (e.g.,
pass graph to boost::cuthill_mckee_ordering / boost::king_ordering /
boost::sloan_ordering without mutating it); ensure any local variables that
previously tried to modify graph are copied or made non-mutating so the
implementations compile with the new const parameter.
In @NEWS.md:
- Around line 19-21: The NEWS.md change is auto-generated and must not be edited
directly; revert the manual edit in NEWS.md, add the "Code enhancement" entry
(including the pgrouting issue link and "Ordering functions standardized" text)
into the source input consumed by the notes2news.pl generator (referenced as
notes2news.pl), then re-run the generator to regenerate NEWS.md so the entry is
preserved via the canonical source instead of a direct edit.
In @src/ordering/ordering_driver.cpp:
- Line 115: Remove the stray extra semicolon after the DirectedGraph constructor
call: in the expression that initializes digraph (using DirectedGraph digraph =
which >= 11 ? DirectedGraph(vertices) : DirectedGraph();; ), drop the second
semicolon so the statement ends with a single semicolon; this touches the
initialization of variable digraph and does not change logic.
- Around line 110-116: The conditional logic using magic numbers on the variable
which (e.g., "which == 0 || which == 2" and "which < 11" / "which >= 11") should
be replaced with explicit comparisons against the Which enum values; update the
vertices extraction and graph constructors to check e.g. which == Which::X or
which == Which::Y and which <= Which::SomeThreshold (or direct enum predicates
like isUndirected(which) / isDirected(which)) so the branches use named enum
constants instead of 0, 2, 11; modify the expressions around
pgrouting::extract_vertices(edges), UndirectedGraph(vertices) and
DirectedGraph(vertices) accordingly so behavior is preserved but driven by Which
enum names.
- Around line 114-115: The code currently constructs both UndirectedGraph
(undigraph) and DirectedGraph (digraph) regardless of the value of which; change
this so only the needed graph is constructed by deferring construction and using
one of: (a) declare std::optional<UndirectedGraph> undigraph and
std::optional<DirectedGraph> digraph and emplace only the one you need, or (b)
use std::unique_ptr<UndirectedGraph>/std::unique_ptr<DirectedGraph> and
reset/new only the chosen graph, or (c) use an if/else to construct a single
local variable with the appropriate type; apply the change where undigraph and
digraph are declared/initialized (referencing the symbols undigraph, digraph,
UndirectedGraph, DirectedGraph, which, vertices) and remove the extra semicolon
after DirectedGraph() to clean up syntax.
In @src/ordering/topologicalSort.cpp:
- Around line 57-58: Replace the throw of a std::string in the catch block with
a proper exception type: change the catch handling in topologicalSort.cpp (the
catch for boost::exception const& ex) to throw a std::runtime_error (or a custom
exception class) with the message "Graph is not DAG", and add #include
<stdexcept> at the top of the file so the type is available; keep the original
catch target (boost::exception const& ex) so you can also include ex.what() or
diagnostic info if desired.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (29)
NEWS.mddoc/src/pgRouting-introduction.rstdoc/src/release_notes.rstinclude/c_common/enums.hinclude/cpp_common/basic_vertex.hppinclude/cpp_common/to_postgres.hppinclude/drivers/ordering/cuthillMckeeOrdering_driver.hinclude/drivers/ordering/topologicalSort_driver.hinclude/drivers/ordering_driver.hppinclude/ordering/cuthillMckeeOrdering.hppinclude/ordering/kingOrdering.hppinclude/ordering/sloanOrdering.hppinclude/ordering/topologicalSort.hppinclude/process/ordering_process.hlocale/en/LC_MESSAGES/pgrouting_doc_strings.polocale/pot/pgrouting_doc_strings.potsrc/ordering/CMakeLists.txtsrc/ordering/cuthillMckeeOrdering.csrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/cuthillMckeeOrdering_driver.cppsrc/ordering/kingOrdering.csrc/ordering/kingOrdering.cppsrc/ordering/ordering_driver.cppsrc/ordering/ordering_process.cppsrc/ordering/sloanOrdering.csrc/ordering/sloanOrdering.cppsrc/ordering/topologicalSort.csrc/ordering/topologicalSort.cppsrc/ordering/topologicalSort_driver.cpp
💤 Files with no reviewable changes (4)
- include/drivers/ordering/cuthillMckeeOrdering_driver.h
- src/ordering/topologicalSort_driver.cpp
- src/ordering/cuthillMckeeOrdering_driver.cpp
- include/drivers/ordering/topologicalSort_driver.h
🧰 Additional context used
🧠 Learnings (53)
📓 Common learnings
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
📚 Learning: 2025-02-06T20:50:07.967Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Applied to files:
doc/src/release_notes.rstinclude/ordering/topologicalSort.hppinclude/ordering/cuthillMckeeOrdering.hppinclude/cpp_common/to_postgres.hppinclude/ordering/sloanOrdering.hppNEWS.mddoc/src/pgRouting-introduction.rstsrc/ordering/topologicalSort.csrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-09-15T00:39:05.272Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: NEWS.md:16-24
Timestamp: 2025-09-15T00:39:05.272Z
Learning: In pgRouting, editorial improvements to NEWS.md content should be implemented via the notes2news.pl script or its source files, since NEWS.md is auto-generated and manual edits would be overwritten.
Applied to files:
doc/src/release_notes.rstNEWS.mddoc/src/pgRouting-introduction.rstsrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:41:25.296Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:56-57
Timestamp: 2025-04-22T22:41:25.296Z
Learning: In pgRouting, NEWS.md is an automatically generated file that should not be modified manually. Changes to the NEWS.md content should be made by modifying the generator script (tools/release-scripts/notes2news.pl) or its source files.
Applied to files:
doc/src/release_notes.rstNEWS.mddoc/src/pgRouting-introduction.rstlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:41:34.709Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: NEWS.md:47-50
Timestamp: 2025-04-22T22:41:34.709Z
Learning: In pgRouting, NEWS.md is a generated file created by the tools/release-scripts/notes2news.pl script and should not be modified manually.
Applied to files:
doc/src/release_notes.rstNEWS.mddoc/src/pgRouting-introduction.rstlocale/pot/pgrouting_doc_strings.pot
📚 Learning: 2025-02-27T23:08:14.497Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_degree.rst:28-35
Timestamp: 2025-02-27T23:08:14.497Z
Learning: In the pgRouting documentation, historical version notes should be retained to show the evolution of functions across different versions.
Applied to files:
doc/src/release_notes.rstNEWS.mddoc/src/pgRouting-introduction.rstlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-27T01:49:25.212Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
Applied to files:
doc/src/release_notes.rstNEWS.mdsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-05-22T21:14:00.661Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2898
File: NEWS.md:99-99
Timestamp: 2025-05-22T21:14:00.661Z
Learning: In pgRouting, the NEWS.md file is automatically generated and should not be manually edited, as changes would be overwritten during the next generation.
Applied to files:
doc/src/release_notes.rstNEWS.mddoc/src/pgRouting-introduction.rst
📚 Learning: 2025-04-24T23:34:23.099Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: NEWS.md:46-47
Timestamp: 2025-04-24T23:34:23.099Z
Learning: For pgRouting, NEWS.md should be updated using the generator script (tools/release-scripts/notes2news.pl) rather than edited directly, which is the correct procedure that the project maintainers follow.
Applied to files:
doc/src/release_notes.rstNEWS.mddoc/src/pgRouting-introduction.rstlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-06T20:50:00.577Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Applied to files:
doc/src/release_notes.rstinclude/ordering/topologicalSort.hppinclude/ordering/cuthillMckeeOrdering.hppinclude/cpp_common/to_postgres.hppinclude/ordering/sloanOrdering.hppNEWS.mdsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpp
📚 Learning: 2025-02-27T23:09:12.162Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: doc/topology/pgr_extractVertices.rst:28-35
Timestamp: 2025-02-27T23:09:12.162Z
Learning: In pgRouting documentation, historical version entries should be maintained in reverse chronological order (newest first) to provide context about the evolution of functions over time.
Applied to files:
doc/src/release_notes.rstinclude/ordering/topologicalSort.hppNEWS.mddoc/src/pgRouting-introduction.rstsrc/ordering/sloanOrdering.cppsrc/ordering/ordering_process.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-02-27T16:42:23.807Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: doc/withPoints/pgr_withPoints.rst:30-35
Timestamp: 2025-02-27T16:42:23.807Z
Learning: For pgr_withPoints, the original function was proposed in version 2.2.0, while the Combinations signature overload was added in version a3.2.0.
Applied to files:
doc/src/release_notes.rst
📚 Learning: 2025-09-14T16:43:16.715Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/boost_version.yml:134-135
Timestamp: 2025-09-14T16:43:16.715Z
Learning: In pgrouting, the installed PostgreSQL extension version is normalized to major.minor.patch format (e.g., "4.0.0") regardless of any pre-release suffixes like -alpha1, -beta, or -rc that are used in the source/build versioning. Database setup scripts should use the normalized version format.
Applied to files:
doc/src/release_notes.rstNEWS.md
📚 Learning: 2025-04-25T15:12:03.608Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2874
File: doc/utilities/pgr_separateCrossing.rst:26-27
Timestamp: 2025-04-25T15:12:03.608Z
Learning: In pgRouting documentation, official functions don't include version information in their index entries (`:single:` directives). Only deprecated functions include when they were deprecated in the index entries.
Applied to files:
doc/src/release_notes.rst
📚 Learning: 2025-01-19T16:19:16.974Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_withPoints_driver.cpp:154-154
Timestamp: 2025-01-19T16:19:16.974Z
Learning: Error messages in pgRouting should be kept minimal and consistent to maintain compatibility with pgtap tests.
Applied to files:
doc/src/release_notes.rstNEWS.mdsrc/ordering/topologicalSort.cppsrc/ordering/topologicalSort.csrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/ordering_driver.cpp
📚 Learning: 2026-01-06T20:27:35.905Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3026
File: src/ordering/sloanOrdering.cpp:46-89
Timestamp: 2026-01-06T20:27:35.905Z
Learning: In pgRouting, the team plans to eventually stop using the base graph class wrappers. Direct Boost Graph Library calls (e.g., `boost::num_vertices(graph.graph)`) are acceptable and preferred over wrapper methods (e.g., `graph.num_vertices()`) as this future-proofs the code against the planned removal of the base class.
Applied to files:
include/ordering/topologicalSort.hppinclude/ordering/cuthillMckeeOrdering.hppinclude/cpp_common/to_postgres.hppinclude/ordering/kingOrdering.hppinclude/ordering/sloanOrdering.hpp
📚 Learning: 2026-01-06T20:27:35.905Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3026
File: src/ordering/sloanOrdering.cpp:46-89
Timestamp: 2026-01-06T20:27:35.905Z
Learning: In C++ code, prefer direct Boost Graph Library calls over wrapper methods when accessing graph properties to future-proof against planned removal of wrapper base classes. Validate by replacing wrappers like graph.num_vertices() with direct BGL calls such as boost::num_vertices(graph.graph) (adjusting to the actual member names in your code). This guidance applies across C++ sources in the repository where similar graph wrappers are used.
Applied to files:
src/ordering/kingOrdering.cppsrc/ordering/topologicalSort.cppsrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cppsrc/ordering/ordering_process.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-09-14T17:29:04.360Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: CMakeLists.txt:96-104
Timestamp: 2025-09-14T17:29:04.360Z
Learning: In pgRouting, BOOST_THREAD_LIBRARIES is referenced in CMakeLists.txt but the project doesn't actually use Boost threading functionality in the source code, so the LINK_LIBRARIES(${BOOST_THREAD_LIBRARIES}) line should be removed as unused legacy code.
Applied to files:
include/ordering/cuthillMckeeOrdering.hppsrc/ordering/topologicalSort.cppsrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-02-27T16:34:57.518Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: sql/sigs/pgrouting--3.8.sig:274-275
Timestamp: 2025-02-27T16:34:57.518Z
Learning: The file `sql/sigs/pgrouting--3.8.sig` is auto-generated by a command and should not be modified manually.
Applied to files:
include/cpp_common/to_postgres.hppNEWS.mdsrc/ordering/topologicalSort.cppdoc/src/pgRouting-introduction.rstsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-12-31T06:01:45.144Z
Learnt from: Mohit242-bit
Repo: pgRouting/pgrouting PR: 3018
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:11-11
Timestamp: 2025-12-31T06:01:45.144Z
Learning: In pgRouting/pgrouting, CI/Weblate may update locale/pot/pgrouting_doc_strings.pot and locale/en/LC_MESSAGES/pgrouting_doc_strings.po in PRs after .rst changes. These auto-generated updates are acceptable and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
NEWS.mdsrc/ordering/sloanOrdering.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T16:32:00.573Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:0-0
Timestamp: 2025-02-27T16:32:00.573Z
Learning: Modifications to locale files in the pgRouting project should be handled through an action commit or through the Weblate translation platform, not via direct edits.
Applied to files:
NEWS.mdlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T16:31:49.972Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:2740-2740
Timestamp: 2025-02-27T16:31:49.972Z
Learning: Modifications to locale files in pgRouting (like .po files) are done through action commits or in weblate, not through direct manual edits in PRs.
Applied to files:
NEWS.mdlocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-10-30T16:43:11.554Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2989
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:10-10
Timestamp: 2025-10-30T16:43:11.554Z
Learning: When a locale translation change (e.g., locale/es/LC_MESSAGES/pgrouting_doc_strings.po) is introduced by a Weblate-authored commit within a PR, it is compliant with pgRouting’s workflow and should not be flagged; only manual edits to .po files should be discouraged.
Applied to files:
NEWS.mdsrc/ordering/sloanOrdering.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T13:16:44.066Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:192-194
Timestamp: 2025-01-19T13:16:44.066Z
Learning: For the pgRouting repository, keep review comments concise and avoid excessive detail in suggestions.
Applied to files:
NEWS.mdsrc/ordering/topologicalSort.cppsrc/ordering/topologicalSort.cinclude/c_common/enums.hsrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-09-14T16:44:12.051Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/check-files.yml:54-55
Timestamp: 2025-09-14T16:44:12.051Z
Learning: In the pgRouting repository, GitHub Actions version updates (like actions/checkout versions) are handled automatically by a robot/automation, so manual version update suggestions are not needed.
Applied to files:
NEWS.md
📚 Learning: 2025-09-14T17:22:22.313Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: .github/workflows/update.yml:132-133
Timestamp: 2025-09-14T17:22:22.313Z
Learning: In pgRouting, when testing extension updates in workflows, the target version should be the normalized major.minor.patch format (e.g., "4.0.0") even for pre-release versions like 4.0.0-alpha1, because the extension is installed with the normalized version number regardless of source version suffixes.
Applied to files:
NEWS.md
📚 Learning: 2025-01-19T16:17:53.496Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/coloring/edgeColoring_driver.cpp:87-87
Timestamp: 2025-01-19T16:17:53.496Z
Learning: Changes to error message handling in pgRouting functions require corresponding updates to pgtap tests to maintain test coverage.
Applied to files:
NEWS.mdsrc/ordering/topologicalSort.cppsrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/ordering_driver.cpp
📚 Learning: 2025-02-27T16:33:56.959Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2764
File: sql/sigs/pgrouting--3.8.sig:125-136
Timestamp: 2025-02-27T16:33:56.959Z
Learning: The file sql/sigs/pgrouting--*.sig is automatically generated by a command and cannot be manually modified. Comments about naming conventions or other improvements should be directed at the source code that generates these signatures, not at the signature file itself.
Applied to files:
src/ordering/topologicalSort.cppdoc/src/pgRouting-introduction.rstsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-02-27T23:05:28.589Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: sql/common/_checkquery.sql:33-33
Timestamp: 2025-02-27T23:05:28.589Z
Learning: When reviewing PostgreSQL/pgRouting code, respect the maintainer's preference for brevity over additional comments when variable names are descriptive.
Applied to files:
src/ordering/topologicalSort.cppsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:18:00.783Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_withPoints_driver.cpp:120-121
Timestamp: 2025-01-19T16:18:00.783Z
Learning: In pgRouting, modifying error messages requires updating pgtap test expectations, so such changes should be carefully considered due to the test maintenance burden.
Applied to files:
src/ordering/topologicalSort.cppsrc/ordering/topologicalSort.csrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:18:09.359Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trsp_driver.cpp:231-232
Timestamp: 2025-01-19T16:18:09.359Z
Learning: In pgRouting, modifications to error message handling need careful consideration as they impact pgtap test cases which verify exact error message content.
Applied to files:
src/ordering/topologicalSort.cppsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/ordering_driver.cpp
📚 Learning: 2025-01-19T16:18:55.529Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/dijkstra/dijkstra_driver.cpp:147-148
Timestamp: 2025-01-19T16:18:55.529Z
Learning: In pgRouting, error messages should not be modified without careful consideration as they are verified by pgtap tests. Changes to error messages require corresponding updates to test cases.
Applied to files:
src/ordering/topologicalSort.cppsrc/ordering/topologicalSort.csrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:38.435Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/pickDeliver/pickDeliver_driver.cpp:69-69
Timestamp: 2025-01-19T16:17:38.435Z
Learning: Error messages in pgRouting are part of pgtap test assertions and should not be modified without updating the corresponding tests.
Applied to files:
src/ordering/topologicalSort.cppsrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.posrc/ordering/ordering_driver.cpp
📚 Learning: 2025-02-27T23:04:38.878Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: sql/common/_checkcolumn.sql:39-53
Timestamp: 2025-02-27T23:04:38.878Z
Learning: The pgRouting codebase uses PostgreSQL's HINT feature in exception handling to provide context about SQL queries, combined with ERRCODE = SQLSTATE to preserve original error codes.
Applied to files:
src/ordering/topologicalSort.cpp
📚 Learning: 2025-01-19T16:17:46.713Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/planar/boyerMyrvold_driver.cpp:100-100
Timestamp: 2025-01-19T16:17:46.713Z
Learning: Changes to error message handling in pgRouting need to consider impact on pgtap tests, as they may have assertions that depend on specific message behavior.
Applied to files:
src/ordering/topologicalSort.cppsrc/ordering/ordering_process.cpp
📚 Learning: 2025-04-29T18:59:06.824Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2879
File: sql/dijkstra/dijkstra.sql:56-57
Timestamp: 2025-04-29T18:59:06.824Z
Learning: The maintainer prefers not to add inline comments to internal function calls like _pgr_dijkstra, even when they contain multiple boolean and numeric flags.
Applied to files:
src/ordering/topologicalSort.csrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.c
📚 Learning: 2025-04-25T00:59:52.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: src/contraction/contractionHierarchies_driver.cpp:47-56
Timestamp: 2025-04-25T00:59:52.580Z
Learning: In pgRouting, driver functions like `pgr_contractionHierarchies` assume that callers within the project will pass valid non-null pointers, as these functions are only used internally with well-defined call patterns. Null checks on the pointers themselves (as opposed to their dereferenced values) are considered unnecessary since the caller is guaranteed to pass references to variables.
Applied to files:
src/ordering/topologicalSort.csrc/ordering/sloanOrdering.cppsrc/ordering/cuthillMckeeOrdering.csrc/ordering/ordering_driver.cpp
📚 Learning: 2025-09-17T02:09:53.993Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: doc/contraction/pgr_contraction.rst:0-0
Timestamp: 2025-09-17T02:09:53.993Z
Learning: In pgRouting documentation, pseudocode blocks can use generic variable names that don't necessarily match the exact parameter names in the API, as decided by maintainer cvvergara for doc/contraction/pgr_contraction.rst.
Applied to files:
src/ordering/topologicalSort.csrc/ordering/ordering_driver.cpp
📚 Learning: 2025-04-24T23:29:43.790Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: tools/testers/contractionHierarchies_data.sql:73-85
Timestamp: 2025-04-24T23:29:43.790Z
Learning: Test data for pgRouting algorithms like contraction hierarchies doesn't necessarily need to follow all production best practices such as setting explicit SRIDs on geometries, especially when the algorithm primarily works with graph topology rather than geographic properties.
Applied to files:
src/ordering/topologicalSort.csrc/ordering/sloanOrdering.cppsrc/ordering/ordering_driver.cpp
📚 Learning: 2025-04-22T22:56:39.572Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: doc/tsp/pgr_TSPeuclidean.rst:55-56
Timestamp: 2025-04-22T22:56:39.572Z
Learning: User cvvergara prefers to handle grammar improvements in separate PRs from spelling/typo corrections.
Applied to files:
include/c_common/enums.h
📚 Learning: 2025-01-19T16:18:49.447Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/bellman_ford/bellman_ford_neg_driver.cpp:119-119
Timestamp: 2025-01-19T16:18:49.447Z
Learning: The edge count check in bellman_ford_neg_driver.cpp is part of code that is not yet in use, as of January 2025.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-09-14T17:29:04.360Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: CMakeLists.txt:96-104
Timestamp: 2025-09-14T17:29:04.360Z
Learning: In pgRouting CMakeLists.txt, the project does not use the Boost thread component, so LINK_LIBRARIES(${BOOST_THREAD_LIBRARIES}) should be removed rather than adding COMPONENTS thread to find_package(Boost).
Applied to files:
src/ordering/cuthillMckeeOrdering.cpp
📚 Learning: 2025-01-19T13:17:02.243Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2723
File: tools/developer/run.sh:62-68
Timestamp: 2025-01-19T13:17:02.243Z
Learning: For pgRouting project, CMake configuration comments should be kept minimal and concise, avoiding extensive explanations of build options and configurations.
Applied to files:
src/ordering/cuthillMckeeOrdering.cpp
📚 Learning: 2025-04-24T23:36:04.580Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2868
File: include/drivers/contraction/contractionHierarchies_driver.h:1-73
Timestamp: 2025-04-24T23:36:04.580Z
Learning: The copyright year for pgRouting should be set to 2025 in new files, not 2024, as confirmed by the project maintainer.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-12-30T19:45:32.986Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 3004
File: .clang-tidy:9-11
Timestamp: 2025-12-30T19:45:32.986Z
Learning: In pgRouting's .clang-tidy file (and similar list configurations), the last line of lists should not have a trailing comma, per project convention.
Applied to files:
src/ordering/cuthillMckeeOrdering.cppsrc/ordering/sloanOrdering.cpp
📚 Learning: 2025-01-19T16:17:31.895Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/contraction/contractGraph_driver.cpp:187-187
Timestamp: 2025-01-19T16:17:31.895Z
Learning: Changes to error messages in pgRouting must carefully consider impacts on pgtap tests, as these tests verify exact message content.
Applied to files:
src/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-01-19T16:17:42.135Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2725
File: src/trsp/trspVia_driver.cpp:151-151
Timestamp: 2025-01-19T16:17:42.135Z
Learning: Modifying error messages in pgRouting requires updating the corresponding pgtap tests to match the new messages, as they verify exact string outputs.
Applied to files:
src/ordering/ordering_process.cpplocale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-02-27T23:03:08.160Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2771
File: locale/en/LC_MESSAGES/pgrouting_doc_strings.po:4407-4419
Timestamp: 2025-02-27T23:03:08.160Z
Learning: In the pgRouting project, locale files (.po files) should not be manually edited as they are managed through automated localization tools.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:59:10.377Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:15446-15454
Timestamp: 2025-04-22T22:59:10.377Z
Learning: En pgRouting, las traducciones al español de las cadenas de documentación (por ejemplo, en `pgrouting_doc_strings.po`) se gestionan a través de Weblate y no se deben sugerir ni modificar directamente en los pull requests.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:58:08.854Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:16304-16305
Timestamp: 2025-04-22T22:58:08.854Z
Learning: En el repositorio pgRouting, las traducciones al español se gestionan exclusivamente a través de Weblate, por lo que no se deben sugerir cambios manuales en los archivos .po para ese idioma en las revisiones de PR.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-09-14T23:32:11.977Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2970
File: doc/CMakeLists.txt:160-161
Timestamp: 2025-09-14T23:32:11.977Z
Learning: The pgRouting project uses Weblate for translation management. The CMake locale target only generates English locale files (`-l en`) because Weblate handles updating all other language translations automatically. This prevents conflicts between local builds and Weblate's translation workflow.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:36:50.711Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/zh_Hans/LC_MESSAGES/pgrouting_doc_strings.po:4083-4087
Timestamp: 2025-04-22T22:36:50.711Z
Learning: For the `locale/zh_Hans/LC_MESSAGES/pgrouting_doc_strings.po` file, Chinese translations should be managed and updated via Weblate, not by direct manual edits or suggestions in the repository.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
📚 Learning: 2025-04-22T22:58:42.373Z
Learnt from: cvvergara
Repo: pgRouting/pgrouting PR: 2864
File: locale/es/LC_MESSAGES/pgrouting_doc_strings.po:4429-4443
Timestamp: 2025-04-22T22:58:42.373Z
Learning: En el proyecto pgRouting, las traducciones al español se gestionan exclusivamente a través de Weblate, por lo que no se deben sugerir cambios de traducción en los pull requests.
Applied to files:
locale/pot/pgrouting_doc_strings.potlocale/en/LC_MESSAGES/pgrouting_doc_strings.po
🧬 Code graph analysis (8)
src/ordering/kingOrdering.cpp (2)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)
include/process/ordering_process.h (1)
src/ordering/ordering_process.cpp (2)
pgr_process_ordering(50-96)pgr_process_ordering(50-54)
src/ordering/topologicalSort.c (1)
src/ordering/ordering_process.cpp (2)
pgr_process_ordering(50-96)pgr_process_ordering(50-54)
src/ordering/cuthillMckeeOrdering.cpp (3)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)src/ordering/sloanOrdering.cpp (2)
colors(60-60)inv_permutation(57-57)
src/ordering/sloanOrdering.cpp (3)
include/cpp_common/base_graph.hpp (1)
num_vertices(599-599)include/dijkstra/drivingDist.hpp (1)
color_map(125-125)src/ordering/cuthillMckeeOrdering.cpp (2)
colors(73-73)inv_permutation(70-70)
src/ordering/cuthillMckeeOrdering.c (1)
src/ordering/ordering_process.cpp (2)
pgr_process_ordering(50-96)pgr_process_ordering(50-54)
src/ordering/ordering_process.cpp (1)
src/common/time_msg.c (1)
time_msg(31-39)
src/ordering/ordering_driver.cpp (5)
include/cpp_common/to_postgres.hpp (2)
get_vertexId(84-97)get_vertexId(84-88)src/ordering/sloanOrdering.cpp (2)
sloanOrdering(46-89)sloanOrdering(47-47)include/ordering/sloanOrdering.hpp (1)
sloanOrdering(41-41)include/ordering/kingOrdering.hpp (1)
kingOrdering(42-42)src/ordering/cuthillMckeeOrdering.cpp (2)
cuthillMckeeOrdering(59-97)cuthillMckeeOrdering(60-60)
🪛 Cppcheck (2.19.0)
src/ordering/kingOrdering.cpp
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 81-81: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'kingOrdering' is never used.
(unusedFunction)
src/ordering/topologicalSort.cpp
[information] 35-35: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 48-48: The function 'topologicalSort' is never used.
(unusedFunction)
src/ordering/topologicalSort.c
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 43-43: The function '_pgr_topologicalsort' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
src/ordering/cuthillMckeeOrdering.cpp
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 81-81: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 60-60: The function 'cuthillMckeeOrdering' is never used.
(unusedFunction)
src/ordering/sloanOrdering.cpp
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 34-34: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
[information] 35-35: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 81-81: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[style] 47-47: The function 'sloanOrdering' is never used.
(unusedFunction)
src/ordering/cuthillMckeeOrdering.c
[information] 35-35: Include file
(missingIncludeSystem)
[information] 36-36: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[information] 39-39: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 37-37: Include file
(missingIncludeSystem)
[information] 38-38: Include file
(missingIncludeSystem)
[style] 42-42: The function '_pgr_cuthillmckeeordering' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
src/ordering/ordering_driver.cpp
[information] 40-40: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 45-45: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingIncludeSystem)
[information] 43-43: Include file
(missingIncludeSystem)
[information] 44-44: Include file
(missingIncludeSystem)
[information] 40-40: Include file
(missingIncludeSystem)
[information] 41-41: Include file
(missingIncludeSystem)
[performance] 81-81: Function parameter 'data_edges' should be passed by const reference.
(passedByValue)
[style] 57-57: The function 'set_ids' is never used.
(unusedFunction)
[style] 71-71: The function 'start_id' is never used.
(unusedFunction)
[style] 73-73: The function 'end_id' is never used.
(unusedFunction)
[style] 75-75: The function 'tot_cost' is never used.
(unusedFunction)
[style] 59-59: The function 'dest_id' is never used.
(unusedFunction)
[style] 63-63: The function 'do_ordering' is never used.
(unusedFunction)
[performance] 113-113: Function parameter 'results' should be passed by const reference.
(passedByValue)
[style] 111-111: The function 'vector_to_tuple' is never used.
(unusedFunction)
[style] 145-145: The function 'matrix_to_tuple' is never used.
(unusedFunction)
| std::vector<pgrouting::UndirectedGraph::V> | ||
| cuthillMckeeOrdering(pgrouting::UndirectedGraph &graph); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check signatures of other ordering functions for consistency
echo "=== Checking kingOrdering signature ==="
ast-grep --pattern $'std::vector<$_> kingOrdering($$$)'
echo ""
echo "=== Checking sloanOrdering signature ==="
ast-grep --pattern $'std::vector<$_> sloanOrdering($$$)'
echo ""
echo "=== Checking topologicalSort signature ==="
ast-grep --pattern $'std::vector<$_> topologicalSort($$$)'
echo ""
echo "=== Checking cuthillMckeeOrdering signature ==="
ast-grep --pattern $'std::vector<$_> cuthillMckeeOrdering($$$)'Repository: pgRouting/pgrouting
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Search for header files containing these ordering functions
echo "=== Finding ordering function headers ==="
fd -t f -e hpp . include/ordering/ 2>/dev/null | head -20
echo ""
echo "=== Contents of cuthillMckeeOrdering.hpp ==="
cat include/ordering/cuthillMckeeOrdering.hppRepository: pgRouting/pgrouting
Length of output: 1794
🏁 Script executed:
#!/bin/bash
# Search more broadly for function signatures
echo "=== Searching for kingOrdering ==="
rg "kingOrdering" -t hpp --max-count 5
echo ""
echo "=== Searching for sloanOrdering ==="
rg "sloanOrdering" -t hpp --max-count 5Repository: pgRouting/pgrouting
Length of output: 207
🏁 Script executed:
#!/bin/bash
# Read the kingOrdering header
echo "=== Contents of kingOrdering.hpp ==="
cat include/ordering/kingOrdering.hpp
echo ""
echo "=== Contents of sloanOrdering.hpp ==="
cat include/ordering/sloanOrdering.hppRepository: pgRouting/pgrouting
Length of output: 3007
🏁 Script executed:
#!/bin/bash
# Search for function signatures using rg with correct syntax
echo "=== Searching for all ordering function declarations ==="
rg "Ordering\(" include/ordering/ -A 1 -B 1
echo ""
echo "=== Check if any use const parameter ==="
rg "const.*UndirectedGraph" include/ordering/ -A 1Repository: pgRouting/pgrouting
Length of output: 788
🏁 Script executed:
#!/bin/bash
# Find and examine the implementation files
echo "=== Finding implementation files for ordering functions ==="
fd -t f -e cpp . src/ordering/ 2>/dev/null | head -20
echo ""
echo "=== Check cuthillMckeeOrdering implementation ==="
fd -t f "cuthillMckee" src/
echo ""
echo "=== Check kingOrdering implementation ==="
fd -t f "king" src/ordering/ 2>/dev/null
echo ""
echo "=== Check sloan implementation ==="
fd -t f "sloan" src/ordering/ 2>/dev/nullRepository: pgRouting/pgrouting
Length of output: 643
🏁 Script executed:
#!/bin/bash
# Look at the actual implementations
echo "=== cuthillMckeeOrdering.cpp ==="
find . -name "*cuthillMckee*.cpp" -exec cat {} \; 2>/dev/null | head -100
echo ""
echo "=== Check if graph is modified in any implementation ==="
find . -path "*/ordering/*.cpp" -exec grep -l "graph\." {} \; 2>/dev/nullRepository: pgRouting/pgrouting
Length of output: 3332
🏁 Script executed:
#!/bin/bash
# Check the other implementations to see if they also only read the graph
echo "=== kingOrdering.cpp ==="
cat src/ordering/kingOrdering.cpp | head -100
echo ""
echo "=== sloanOrdering.cpp ==="
cat src/ordering/sloanOrdering.cpp | head -100
echo ""
echo "=== Check if UndirectedGraph methods used in implementations are const-friendly ==="
rg "num_vertices|vertex_index|make_out_degree_map|make_iterator_property_map" include/cpp_common/ -A 2 | head -50Repository: pgRouting/pgrouting
Length of output: 8994
🏁 Script executed:
#!/bin/bash
# Check if the graph member variable is const-accessible
echo "=== Check base_graph.hpp for const-correctness support ==="
cat include/cpp_common/base_graph.hpp | head -200
echo ""
echo "=== Search for const usage in ordering algorithms ==="
rg "const.*UndirectedGraph" src/ordering/ -A 2Repository: pgRouting/pgrouting
Length of output: 5905
🏁 Script executed:
#!/bin/bash
# Verify Boost graph algorithms accept const references
echo "=== Check Boost algorithm signatures ==="
web_search "boost::cuthill_mckee_ordering const graph reference"Repository: pgRouting/pgrouting
Length of output: 171
🌐 Web query:
boost::cuthill_mckee_ordering const reference parameter requirements
💡 Result:
Short answer: the graph parameter may be a const reference (signatures use const Graph& or const IncidenceGraph&). The graph type must model the required graph concepts (IncidenceGraph for the single-component APIs, or VertexListGraph+IncidenceGraph for the vertex-list variants). In addition you must supply appropriate property maps: a writable/work ColorMap (used internally), and a DegreeMap (a readable Vertex->degree map) or a VertexIndexMap for the overloads that infer maps. See Boost Graph Library docs for exact signatures and parameter requirements. [1][2]
Sources:
- Boost Graph Library — Cuthill–McKee ordering (function signatures and parameter notes). [1]
- boost/graph/cuthill_mckee_ordering.hpp (implementation / overloads showing const Graph& signatures). [2]
Add const qualifier to graph parameter for all three ordering functions.
The function signatures are consistent across cuthillMckeeOrdering, kingOrdering, and sloanOrdering, all using the same non-const parameter signature. However, the parameter should be const pgrouting::UndirectedGraph &graph since these implementations only read the graph structure and the underlying Boost graph algorithms (boost::cuthill_mckee_ordering, boost::king_ordering, boost::sloan_ordering) support const references. Update all three function declarations and implementations to use the const qualifier for improved const-correctness.
🤖 Prompt for AI Agents
In @include/ordering/cuthillMckeeOrdering.hpp around lines 41 - 42, Change the
three ordering functions to accept the graph by const reference: update the
declarations and definitions of cuthillMckeeOrdering, kingOrdering, and
sloanOrdering to use const pgrouting::UndirectedGraph &graph instead of a
non-const reference, and adjust any internal calls if necessary to use
const-compatible Boost algorithms (e.g., pass graph to
boost::cuthill_mckee_ordering / boost::king_ordering / boost::sloan_ordering
without mutating it); ensure any local variables that previously tried to modify
graph are copied or made non-mutating so the implementations compile with the
new const parameter.
| Code enhancement | ||
|
|
||
| * [#3025](https://github.com/pgRouting/pgrouting/issues/3025): Ordering functions standardized |
There was a problem hiding this comment.
NEWS.md is auto-generated and should not be edited manually.
The changes to NEWS.md (lines 19–21) modify an auto-generated file. Per pgRouting's workflow, changes to NEWS.md content should be implemented via the generator script tools/release-scripts/notes2news.pl or its source files, not by direct manual edits. Manual changes will be overwritten during the next generation.
Please move the "Code enhancement" entry to the source file that feeds the notes2news.pl script instead of editing NEWS.md directly.
🤖 Prompt for AI Agents
In @NEWS.md around lines 19 - 21, The NEWS.md change is auto-generated and must
not be edited directly; revert the manual edit in NEWS.md, add the "Code
enhancement" entry (including the pgrouting issue link and "Ordering functions
standardized" text) into the source input consumed by the notes2news.pl
generator (referenced as notes2news.pl), then re-run the generator to regenerate
NEWS.md so the entry is preserved via the canonical source instead of a direct
edit.
| auto vertices = which == 0 || which == 2? | ||
| pgrouting::extract_vertices(edges) | ||
| : std::vector<pgrouting::Basic_vertex>(); | ||
|
|
||
| UndirectedGraph undigraph = which < 11? UndirectedGraph(vertices) : UndirectedGraph(); | ||
| DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();; | ||
|
|
There was a problem hiding this comment.
Replace magic numbers with enum comparisons.
The code uses magic numbers (0, 2, 11) to determine graph construction, making it brittle and hard to maintain. If the Which enum values change, this code will break silently.
🔧 Proposed fix using enum values
- auto vertices = which == 0 || which == 2?
+ auto vertices = (which == SLOAN || which == CUTCHILL || which == KING)?
pgrouting::extract_vertices(edges)
: std::vector<pgrouting::Basic_vertex>();
- UndirectedGraph undigraph = which < 11? UndirectedGraph(vertices) : UndirectedGraph();
- DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();;
+ UndirectedGraph undigraph = (which == SLOAN || which == CUTCHILL || which == KING)?
+ UndirectedGraph(vertices) : UndirectedGraph();
+ DirectedGraph digraph = (which == TOPOSORT)?
+ DirectedGraph(vertices) : DirectedGraph();Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Cppcheck (2.19.0)
[performance] 113-113: Function parameter 'results' should be passed by const reference.
(passedByValue)
[style] 111-111: The function 'vector_to_tuple' is never used.
(unusedFunction)
🤖 Prompt for AI Agents
In @src/ordering/ordering_driver.cpp around lines 110 - 116, The conditional
logic using magic numbers on the variable which (e.g., "which == 0 || which ==
2" and "which < 11" / "which >= 11") should be replaced with explicit
comparisons against the Which enum values; update the vertices extraction and
graph constructors to check e.g. which == Which::X or which == Which::Y and
which <= Which::SomeThreshold (or direct enum predicates like
isUndirected(which) / isDirected(which)) so the branches use named enum
constants instead of 0, 2, 11; modify the expressions around
pgrouting::extract_vertices(edges), UndirectedGraph(vertices) and
DirectedGraph(vertices) accordingly so behavior is preserved but driven by Which
enum names.
| UndirectedGraph undigraph = which < 11? UndirectedGraph(vertices) : UndirectedGraph(); | ||
| DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider constructing only the required graph.
Both UndirectedGraph and DirectedGraph are constructed regardless of which, but only one is used. This wastes memory and initialization time.
♻️ More efficient approach
- UndirectedGraph undigraph = which < 11? UndirectedGraph(vertices) : UndirectedGraph();
- DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();;
-
-
switch (which) {
case SLOAN: {
+ UndirectedGraph undigraph(vertices);
process(edges, undigraph, &sloanOrdering, *return_count, return_tuples);
break;
}
case CUTCHILL: {
+ UndirectedGraph undigraph(vertices);
process(edges, undigraph, &cuthillMckeeOrdering, *return_count, return_tuples);
break;
}
case KING: {
+ UndirectedGraph undigraph(vertices);
process(edges, undigraph, &kingOrdering, *return_count, return_tuples);
break;
}
case TOPOSORT: {
+ DirectedGraph digraph(vertices);
process(edges, digraph, &topologicalSort, *return_count, return_tuples);
break;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/ordering/ordering_driver.cpp around lines 114 - 115, The code currently
constructs both UndirectedGraph (undigraph) and DirectedGraph (digraph)
regardless of the value of which; change this so only the needed graph is
constructed by deferring construction and using one of: (a) declare
std::optional<UndirectedGraph> undigraph and std::optional<DirectedGraph>
digraph and emplace only the one you need, or (b) use
std::unique_ptr<UndirectedGraph>/std::unique_ptr<DirectedGraph> and reset/new
only the chosen graph, or (c) use an if/else to construct a single local
variable with the appropriate type; apply the change where undigraph and digraph
are declared/initialized (referencing the symbols undigraph, digraph,
UndirectedGraph, DirectedGraph, which, vertices) and remove the extra semicolon
after DirectedGraph() to clean up syntax.
| : std::vector<pgrouting::Basic_vertex>(); | ||
|
|
||
| UndirectedGraph undigraph = which < 11? UndirectedGraph(vertices) : UndirectedGraph(); | ||
| DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove extra semicolon.
Line 115 has a double semicolon at the end.
🔧 Fix
- DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();;
+ DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph();; | |
| DirectedGraph digraph = which >= 11? DirectedGraph(vertices) : DirectedGraph(); |
🤖 Prompt for AI Agents
In @src/ordering/ordering_driver.cpp at line 115, Remove the stray extra
semicolon after the DirectedGraph constructor call: in the expression that
initializes digraph (using DirectedGraph digraph = which >= 11 ?
DirectedGraph(vertices) : DirectedGraph();; ), drop the second semicolon so the
statement ends with a single semicolon; this touches the initialization of
variable digraph and does not change logic.
| } catch (boost::exception const& ex) { | ||
| throw std::string("Graph is not DAG"); |
There was a problem hiding this comment.
Throw proper exception type instead of std::string.
Throwing std::string (a non-exception type) is bad practice in C++. It bypasses the standard exception hierarchy and can cause issues with exception handling. Use std::runtime_error or a custom exception class instead.
🔧 Proposed fix
} catch (boost::exception const& ex) {
- throw std::string("Graph is not DAG");
+ throw std::runtime_error("Graph is not DAG");
} catch (std::exception &e) {Don't forget to include <stdexcept> at the top of the file:
#include <vector>
#include <algorithm>
#include <string>
+#include <stdexcept>
#include <cstdint>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (boost::exception const& ex) { | |
| throw std::string("Graph is not DAG"); | |
| } catch (boost::exception const& ex) { | |
| throw std::runtime_error("Graph is not DAG"); |
🤖 Prompt for AI Agents
In @src/ordering/topologicalSort.cpp around lines 57 - 58, Replace the throw of
a std::string in the catch block with a proper exception type: change the catch
handling in topologicalSort.cpp (the catch for boost::exception const& ex) to
throw a std::runtime_error (or a custom exception class) with the message "Graph
is not DAG", and add #include <stdexcept> at the top of the file so the type is
available; keep the original catch target (boost::exception const& ex) so you
can also include ex.what() or diagnostic info if desired.
2 participants
Fixes #3025
Rewrite src/ordering/ordering_driver.cpp
to_postgres
(C++) No need of template
Using ordering_driver:
Removing unused files
Adding release notes and NEWS
Adjusting build to changes
@pgRouting/admins
Summary by CodeRabbit
New Features
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.