Skip to content

Limit scope of template aliases - #626

Merged
nkoukpaizan merged 4 commits into
developfrom
slaven/template-aliases
Oct 22, 2026
Merged

nkoukpaizan merged 4 commits into
developfrom
slaven/template-aliases

Conversation

@pelesh

@pelesh pelesh commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Make template aliases private whenever possible. We create template aliases to make the code more readable and to propagate types from the base class to the derived classes. The aliases are intended to be used locally within a class.

Proposed changes

Make template aliases private and document exceptions where an alias needs to be public.

Checklist

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • N/A The CHANGELOG.md has been updated to reflect the changes. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

@pelesh
pelesh requested a review from nkoukpaizan October 20, 2026 13:35
@pelesh pelesh self-assigned this Oct 20, 2026
@pelesh pelesh added the cleanup label Oct 20, 2026
@nkoukpaizan

Copy link
Copy Markdown
Collaborator

I like the approach overall! The read the docs failure looks unrelated to this PR.

There are a few more private/public/trait spots that need to be updated. I'm getting the following compilation errors with -DGRIDKIT_ENABLE_ZMQ=On.

GridKit/examples/Network/CoSim/CoSimClient.cpp:46:47: error: 'SignalNodeT' is a private
      member of 'GridKit::PhasorDynamics::SystemModel<double, unsigned long>'
   46 |   using SignalNodeT  = typename SystemModelT::SignalNodeT;
      |                                               ^
GridKit/examples/Network/CoSim/CoSimClient.cpp:242:17: note: in instantiation of
      template class 'CoSimClient<double, unsigned long>' requested here
  242 |   auto client = CoSimClient<ScalarT, IdxT>(sys,
      |                 ^
GridKit/GridKit/Model/PhasorDynamics/SystemModel.hpp:69:13: note: declared private here
   69 |       using SignalNodeT    = SignalNodeSetT::SignalNodeT;
      |             ^
1 error generated.

@read-the-docs-community

read-the-docs-community Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Documentation build overview

📚 GridKit | 🛠️ Build #35279106 | 📁 Comparing 5126f69 against latest (4315301)

  🔍 Preview build  

77 files changed · + 7 added · ± 59 modified · - 11 deleted

+ Added

± Modified

- Deleted

@pelesh

pelesh commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

I believe I addressed comments from the review. @superwhiskers, I fixed warnings in the co-sim example. Please check if the fix is appropriate.

@pelesh pelesh mentioned this pull request Oct 22, 2026
5 tasks done
@superwhiskers

Copy link
Copy Markdown
Collaborator

seems fine to me. that flag wasn't used at all, i just didn't remove it when modifying the code to get it to work.

@nkoukpaizan
nkoukpaizan merged commit c9a6497 into develop Oct 22, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants