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

Hv iox refactor #129

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

Hv iox refactor #129

wants to merge 43 commits into from

Conversation

mjkpolo
Copy link
Contributor

@mjkpolo mjkpolo commented Jan 17, 2021

Description

converted the hv_iox.c file to cpp and update references accordingly

Type of change

refactoring to cpp

Problem

There are still warnings that were outside the scope of this feature

Copy link
Member

@EUdds EUdds left a comment

Choose a reason for hiding this comment

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

Overall great work! There are a few things I mentioned in the comments that you should touch up on. Otherwise, we'll wait to merge this with main until we get some hardware and can test the code more completely.

@@ -0,0 +1,69 @@
// Author: Marco
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps add some comments to these tests so that someone who doesn't know what the functions are can tell what exactly is failing/passing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👌

if (this->setupIox() != 0)
isInit = false;
}
if (isInit)
Copy link
Member

Choose a reason for hiding this comment

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

An if/else statement here would be less confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it

@@ -0,0 +1,10 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

What is the point of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file generates a compile_commands.json file for the ccls language server. I use this for syntax checking and autocomplete. It also generates other crap I don't need which is the point of the git clean command. I also will run this command as a lazy way to fix build issues if there are untracked files messing up compilation.

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.

2 participants