Style changes to buildAutoChooser #862
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changed filtering function from const PathPlannerAuto* const -> const PathPlannerAuto&:Easier syntax, ref act like varible instead of pointer const_cast<> can still change the const-ness of PathPlannerAuto& which allows them to edit directly Replace dynamic_cast<> to static_cast<> since we know relationship at compile time Set filter as first parameter and defaultAutoName as second (which is defaulted), which prevents confusion as to why there is an empty string input Separated buildAutoChooser into buildAutoChooser, buildAutoChooserFilter, and buildAutoChooserFilterPath for different degrees of concern. * Person who just wants to get a SendableChooser of all PathPlannerAutos just needs to worry about buildAutoChooser * Person who wants to filter out some PathPlannerAutos just needs to worry about buildAutoChooserFilter * Person who wants to filter out some PathPlannerAutos bases on filepath needs to worry about buildAutoChooserFilterPath Updated function descriptions
github-actions
bot
added
PathPlannerLib
|
Tiniest nitpick ever but it looks like a few of the function descriptions have a typo in “recurively” |
|
Yeah you change it to "recursively". That is what happens when spell check isn't there for you. |
|
PPLib-macOS is commented out in in pplib-ci.yml so that is why it not finishing. I do not know if that is intentional but that is why the workflow isn't completing (since it also marked as require). |
|
Yeah I meant to merge this earlier and forgot. macOS is disabled because it fails to build with 2024 WPILib for whatever reason. Works fine with 2025 so I just disabled it temporarily. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changed filtering function from
const PathPlannerAuto* const->const PathPlannerAuto&: Easier syntax, reference act like varible instead of pointerconst_cast<>can still change the const-ness ofPathPlannerAuto&which allows them to edit directly (I despise this feature, however there is no copy constructor forPathPlannerAutodue tounique_ptr)Replace
dynamic_cast<>tostatic_cast<>since we know relationship at compile timeSet
filteras first parameter anddefaultAutoNameas second (which is defaulted), which prevents confusion as to why there is an empty string input (in user code)Separated
buildAutoChooserintobuildAutoChooser,buildAutoChooserFilter, andbuildAutoChooserFilterPathfor different degrees of concern and to mirror java version more neatly (allows people to change from both languages)Updated function descriptions
Degrees of concern for
buildAutoChooserSendableChooserof all PathPlannerAutos just needs to worry aboutbuildAutoChooserfilterout some PathPlannerAutos just needs to worry aboutbuildAutoChooserFilterfilterout some PathPlannerAutos based on filepath (std::filesystem::pathlibary) needs to worry aboutbuildAutoChooserFilterPathI know I have made a lot of changes to this feature however I just want it to be right before backward compatibility requirements happen. I am probably not going to edit this any further as it performs all the roles it need to Java version while maintaining C++ style.
Good luck rolling out for the 2025 season btw👍