Skip to content

Inline global constants and simplify issue lookup - #1475

Open
agarny wants to merge 7 commits into
cellml:mainfrom
agarny:issue1474
Open

agarny wants to merge 7 commits into
cellml:mainfrom
agarny:issue1474

Conversation

@agarny

@agarny agarny commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #1474.

Copilot AI lite review requested due to automatic review settings September 28, 2026 06:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

This PR inlines shared constants and simplifies issue metadata lookup while preserving existing behavior.

Changes:

  • Converts shared constants to C++20 inline variables.
  • Replaces map-based issue lookup with a compact array.
  • Preserves issue headings and URL generation.
File Description
src/​utilities.h Makes shared utility constants inline.
src/​issue.cpp Simplifies issue metadata storage and lookup.
src/​internaltypes.h Makes ORIGIN_MODEL_REF inline.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

issue.cpp must safely handle or define metadata for all supported rule values.

Review effort: Lite
Findings: 1 High severity

Open (1)

Comment thread src/issue.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved review comments remain.

Review effort: Lite
Findings: None

Resolved since last review (1)

@agarny
agarny requested review from hsorby and nickerso September 28, 2026 10:26
Comment thread src/issue.cpp Outdated
Note that it's not a gtest because `ruleToInformation` is file-static in `issue.cpp` and `Issue`'s constructor and `IssueImpl` are private.
hsorby
hsorby previously approved these changes Sep 29, 2026

@hsorby hsorby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a nice improvement.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The only remaining feedback is a non-blocking CMake warning nit.

Review effort: Lite
Findings: 1 Low severity

Open (1)

Comment thread cmake/checkissuereferencerules.cmake Outdated
Refactored the regex handling for issue reference rules to use string matching.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inline global constants and simplify issue lookup

3 participants