Skip to content

Format library #18

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

Merged
merged 8 commits into from
Jan 4, 2024
Merged

Format library #18

merged 8 commits into from
Jan 4, 2024

Conversation

sfe-SparkFro
Copy link
Collaborator

Will resolve #9

I have not yet changed any names, just ran the files through a formatter. @gigapod, you mentioned wanting to change some names away from snake case. I'm also not sold on all the names I chose. Do you have any suggestions?

Clang formatter in VS Code set to Microsoft style
Clang formatter in VS Code set to Microsoft style
@sfe-SparkFro
Copy link
Collaborator Author

Also, are the file names and directory structure okay? If we're going to change those, would probably be best to do that as part of this PR. And any other house-cleaning things like that, really.

Copy link
Member

@gigapod gigapod left a comment

Choose a reason for hiding this comment

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

Overall looks good. The only thing to tweak is the class names - see comment I left.

@PaulZC
Copy link
Contributor

PaulZC commented Dec 15, 2023

u-blox is preferred.
ublox 2nd choice.
(They prefer without capitalisation....)

Many command and response arrays are fixed length, so no need to use calloc. This helps prevent the heap from getting fragmented.
clang with Microsoft style
@sfe-SparkFro
Copy link
Collaborator Author

@gigapod @PaulZC Changes pushed. Also decided to address #12 in this PR, and I removed a bunch of calls to ubx_cell_calloc_char() in favor of putting arrays on the stack; should help with preventing the heap from getting fragmented, and give a minor performance improvement.

Please take a look and let me know what you think! If all is okay, I'd like to get the first release out this week if possible. There's still a couple open issues, but it should at least be totally useable.

@sfe-SparkFro
Copy link
Collaborator Author

@PaulZC @gigapod Last call for input before I merge this early next week. Thanks!

Copy link
Member

@gigapod gigapod left a comment

Choose a reason for hiding this comment

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

Looks good - just one very minor comment/nit.

@sfe-SparkFro sfe-SparkFro merged commit 13ffd73 into v1.0.0 Jan 4, 2024
@sfe-SparkFro sfe-SparkFro deleted the format_library branch January 4, 2024 19:53
@sfe-SparkFro sfe-SparkFro mentioned this pull request Jan 9, 2024
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.

3 participants