search

opendroneid / opendroneid-core-c

Open Drone ID Core C Library
Apache License 2.0
178 stars 64 forks source link

Refactor API and prepend odid_ prefixes #13

Closed hubpav closed 3 years ago

friissoren commented 5 years ago

Thanks for the proposal. These are indeed rather large changes since most types, functions etc. are renamed. I see your point of using consistent naming though, instead of the rather inconsistent usage of CamelCase and underscores currently.

One thing though. This PR will very clearly conflict with PR #14 that I just uploaded. I would prefer if we get that one in first (since it follows the old naming scheme and add several new types etc.) and then after that create a PR that renames things.

I would also like to hear Gabriel's and others opinion on this PR. It will require quite a lot of renaming in any implementation currently using opendronid-core-c and I don't really know how many of those implementations currently exist => how many people this will affect.

Also, we should stay consistent with the Mavlink message implementation: https://mavlink.io/en/messages/common.html#OPEN_DRONE_ID_BASIC_ID https://mavlink.io/en/messages/common.html#MAV_ODID_IDTYPE I don't think all of your current proposals does that.

hubpav commented 5 years ago

Thanks for the review. As stated in the email, the 100% refactoring has not been done yet, only prefixing and public API calls/types have been refactored. I just need to align with you and continue. I definitely agree there should be one unified naming convention which I propose to be underscores :)

It will be tough merging #14 and #13 so yes, merge #14 first and then I have no problem starting over on top of that. I will be quite fast with this.

Speaking compatibility - I think even if there are some users of the API, porting the code to new names should be quite effortless.

Thanks. P.

gabrielcox commented 5 years ago

We need to have a discussion around this. These aren't just prepends, but a total change in style Bumpy's vs all lowers w/underscores, "_t" suffixes etc. I'm not against the prepends (good idea) to be consistent with other prepends, but I don't think we should take a full change in style in a PR without being deliberate in the style of the library. We probably should have clarified the style/consistency up front. I'll first meet with Soren and review the style along with any inconsistencies (and document the style required of the project -- our fault for not doing so already).