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

akit to main #299

Merged
merged 94 commits into from
Sep 26, 2024
Merged

akit to main #299

merged 94 commits into from
Sep 26, 2024

Conversation

Oliver-Cushman
Copy link
Contributor

for the past month akit has basically been main so lets seal the deal

Oliver-Cushman and others added 30 commits July 30, 2024 22:31
after thinking i think this implementation of adv kit works best for terry, where we ignore the separate hardware implementations for real and sim but rather combine the single implementation with the subsystem control logic. its wacky but comparing out ampper code with 6328 code should make it make sense. also not final :P
…a be so small every time but might as well distinguish the actual inputs ig
@Jacob1010-h
Copy link
Member

I love these commit messages 😂 they are beautiful ❤️

@GalexY727
Copy link
Member

someone understood the assignment

@GalexY727
Copy link
Member

GalexY727 commented Sep 11, 2024

A couple notes before this gets merged, however...
Akit is by no means a tiny change. When I made Jerome command-based in the offseason he seemed to work good enough, but I didn't have the same level of quality control when coding him, as it was primarily for a learning experience. Aka, I didn't run that code in any of our off season competitions. If this is to be merged, it means that you're basically locked in for comp to use akit. I recommend you at least make a backup branch of main called pre-akit to make sure that you can always fall back during competition if you run into any sort of bugs that you can't explain. If this isn't reviewed by someone else by Friday I'll review it :). Well done with the upgrade!

@Jacob1010-h
Copy link
Member

I 100% agree with Alex here. For something that is this big of a change it should go through multiple reviewers, or at least be able to be easily backed up in case of something terrible.

I also recommend that all assigned reviewers review the code ASAP. 1. To get familiar with the changes. And 2. To double check multiple times over. At the end of that, then asking for mine or Alex's final review, of which if there is another major change, the process should repeat.

I know it sounds tedious, but I highly recommend it for something this big and new.

Copy link
Contributor

@RudyG252 RudyG252 left a comment

Choose a reason for hiding this comment

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

After reading through every single line of code very in depth and completely and absolutely locking in for hours upon hours, I have determine that this is // working

@Oliver-Cushman
Copy link
Contributor Author

After reading through every single line of code very in depth and completely and absolutely locking in for hours upon hours, I have determine that this is // working

Thank you Zachary Good

@RudyG252
Copy link
Contributor

After reading through every single line of code very in depth and completely and absolutely locking in for hours upon hours, I have determine that this is // working

Thank you Zachary Good

Thank you Clint Cushman

@Oliver-Cushman Oliver-Cushman merged commit a4634dc into main Sep 26, 2024
1 check passed
@Oliver-Cushman Oliver-Cushman deleted the advantagekit branch September 26, 2024 02:31
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.

5 participants