-
Notifications
You must be signed in to change notification settings - Fork 2
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
ROUTING! #7
Conversation
Great work! I just went through and commented on some low hanging fruit. So that the rest of the team can better understand some of the more dense sections, some more in code comments may be nice (Chirp Router and RadioPacket for example). That way we can comment on the functionality of various functions without having to figure out what is happening. Thanks for getting this working! HUGEEE moment! |
Same for me as Spencer, looks good! I'd take a look through for newlines at the ends of files but otherwise great work! |
Some comments or maybe a quick code walkthrough would be helpful, i agree with tim and spencer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything now looks good to me. Some TODO's came out of this PR though, so I will create some github issues and link them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
adding routing! 馃檶