Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Rewrite BehaviorMessage classes to use member initialization, preferred member naming conventions, and const-ref getters #1456

Merged
merged 5 commits into from
Feb 18, 2024

Conversation

jadebenn
Copy link
Collaborator

Changes split out from #1452 for more focused PR.

Copy link
Collaborator Author

@jadebenn jadebenn left a comment

Choose a reason for hiding this comment

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

This leaves out the use of unique pointers, use of string_views, and reduction of raw pointers featured in #1452 in order to minimize the (unfortunately still large) amount of code touched. I will probably split each of those aforementioned points into individual PRs as well after this one is or isn't approved.

@jadebenn jadebenn changed the title refactor: Rewrite BehaviorMessage classes to use member initialization and const-ref getters refactor: Rewrite BehaviorMessage classes to use member initialization, preferred member naming conventions, and const-ref getters Feb 11, 2024
@jadebenn
Copy link
Collaborator Author

Whoops, sorry about the inadvertent assignment there. Wrong button.

aronwk-aaron
aronwk-aaron previously approved these changes Feb 11, 2024
@jadebenn
Copy link
Collaborator Author

I understand if there's reluctance to go through this because of its size, but this was about the most focused I could make these changes. It's just three things (member naming, ordering, and functions) altered over that amount of files.

@aronwk-aaron aronwk-aaron merged commit b6af92e into main Feb 18, 2024
4 checks passed
@aronwk-aaron aronwk-aaron deleted the AMFMemberInitializerLists branch February 18, 2024 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants