team401 / high-key-2024

Other
2 stars 0 forks source link

Initial pre-implementation cleanup #21

Closed aidnem closed 1 month ago

aidnem commented 2 months ago

This PR fixes #18 Purpose The purpose of this addition is to make sure the repository is structured well and isn't too janky before we start adding more specific/advanced features to subsystems.

Project Scope

aidnem commented 1 month ago

@jkleiber I have a code style question: In 2024 we created the CommandSwerveDrivetrain in the tuner constants, and then used a ternary to choose whether or not to assign it in RobotContainer. For high-key I changed this to only create the drivetrain if drive is enabled, but this meant I had to make all of the parameters for drive public so that they could be imported into robot container. Do you have a preference either way on this? I lean towards not instantiating it unless we need it, but I'm open to changing my mind.

jkleiber commented 1 month ago

@aidnem I think the concept of "don't instantiate what you aren't using" makes sense. I'm not sure why you would need to make things public though

aidnem commented 1 month ago

I've fixed the change so I'm going to branch off of here to start integration today since it's the closest we have to a good starting point.

aidnem commented 1 month ago

@jkleiber not sure if you saw during your time away, but we're currently under a boil water advisory, make sure you boil tap water before drinking it or drink bottled water. Figured I'd let you know just in case you didn't get notified while you were away.

jkleiber commented 1 month ago

@aidnem thanks for the heads up - I was aware due to various email updates but better to be safe than sorry 🫡