refactor!: Strongly typed enum for ConstrainedStep::Type and capital initial letter
#4013
Conversation
…l initial letter
WalkthroughSignificant refactoring of the Changes
Sequence DiagramsequenceDiagram
participant CS as ConstrainedStep
participant Stepper as Stepper
participant Propagator as Propagator
CS->>CS: Define Type as scoped enum
Stepper->>CS: Request step size value
CS-->>Stepper: Return value based on Type
Propagator->>Stepper: Update/Release step size
Stepper->>CS: Modify step size with specific Type
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…d capital initial letter" This reverts commit 4070506.
andiwand
changed the title
ConstrainedStep::Type and capital initial letterConstrainedStep::Type
github-actions
bot
added
Component - Core
andiwand
changed the title
ConstrainedStep::Type
andiwand
changed the title
ConstrainedStep::Type and capital initial letter
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Core/include/Acts/Propagator/ConstrainedStep.hpp (1)
65-68: Documentation improvements needed, I sense.Parameter renaming from
valuetov, more concise it makes the code. Yet, in documentation comments, the old parameter namevalueremains. Update these, we must.Apply these changes, you should:
- /// @param value is the new accuracy value + /// @param v is the new accuracy value- /// @param value is the new user value + /// @param v is the new user value- /// @param value is the new value to be updated + /// @param v is the new value to be updatedAlso applies to: 74-78, 114-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Core/include/Acts/Propagator/ConstrainedStep.hpp(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: merge-sentinel
- GitHub Check: macos
🔇 Additional comments (2)
Core/include/Acts/Propagator/ConstrainedStep.hpp (2)
162-163: Clean and elegant, this helper function is!Private helper
value_, encapsulation it brings. Safe enum conversion throughtoUnderlying, wisdom it shows. Maintainable and clear, the code becomes.
50-51: Wise choice, the path to type safety is! Hmmmm.Strong with the Force, this change to
enum class Typeis. Type safety it brings, and proper naming conventions it follows. Theusing enum Typedirective, elegant solution it provides, yes.Verify the impact of enum class conversion, we must:
✅ Verification successful
Safe and sound, this change is! Hmmmm.
Through the Force, seen the truth I have. Properly scoped, all enum values are. No disturbance in the code, there is. Type safety achieved without breaking changes, it has. Strong with the Force, this implementation is!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential implicit conversions that might break rg "ConstrainedStep::(navigator|actor|user)" --type cppLength of output: 57
Script:
#!/bin/bash # Search for ConstrainedStep::Type usage rg "ConstrainedStep::Type" --type cpp -B 1 -A 1 # Search for method calls that might use the enum rg "\.update\(" --type cpp -g "*/Propagator/*" # Search for the class definition to understand usage patterns ast-grep --pattern 'class ConstrainedStep { $$$ }'Length of output: 11477
Script:
#!/bin/bash # Search for specific enum value usage patterns rg "Type::(Navigator|Actor|User)" --type cpp -B 1 -A 1 # Check for potential comparison operations rg "(==|!=|switch.*Type)" --type cpp -g "*/Propagator/*" -B 1 -A 1Length of output: 1002
|
Looks like |
…strainedstep-strongenum+cap
|
acts-project-service
added
the
Fails Athena tests
Followup from #3986
ConstrainedStep::TypeTypeinstancesSummary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
Code Improvement
ConstrainedStepenumeration to use scoped enum (enum class) for improved type safety.Navigator,Actor,User).Type System Changes
ConstrainedStep::Type::Navigator/Actor/Userinstead of previous flat enum references.Compatibility
The changes represent a systematic refactoring to improve code quality and type safety across the project's propagation and stepping mechanisms.