pine64 / bl_iot_sdk

BL602 SDK (Pine64 fork)
https://pine64.github.io/bl602-docs/
Apache License 2.0
134 stars 59 forks source link

Force -Wall & -Werror, fix build warnings #82

Closed konradybcio closed 3 years ago

robertlipe commented 3 years ago

+1 to -Werror. The rest of this overlaps heavily with https://github.com/pine64/bl_iot_sdk/pull/33 which has been in code jail for over a month. I approve of merging them, but we need to work together and quit duplicating effort.

konradybcio commented 3 years ago

So.. should we perhaps get #33 merged and then only merge my Werror/Wall enablement commit so that it builds properly?

robertlipe commented 3 years ago

Yes. Id hope that after committing the first that very little of the second remains. (I think I saw you fix an additional data type to avoid a cast in DAC or something...)

It's been held "for testing" without definition of what that testing is. They're static functions that have no callers. They were seemingly left after a copy/paste with the callers removed. Since they're static, they're known to not have callers. I don't know why these are stuck.

My hardware arrived just yesterday, but there's no working flash tool for MacOS yet. It's not clear what the required acceptance test is as I don't think we have a test suite...

On Tue, Dec 1, 2020, 12:22 PM Konrad Dybcio notifications@github.com wrote:

So.. should we perhaps get #33 https://github.com/pine64/bl_iot_sdk/pull/33 merged and then only merge my Werror/Wall enablement commit so that it builds properly?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pine64/bl_iot_sdk/pull/82#issuecomment-736733029, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD3YYNI2UJWAYGIC5RSDSSUX4PANCNFSM4UIHNLFA .

konradybcio commented 3 years ago

(bump)

konradybcio commented 3 years ago

(bump?)

gamelaster commented 3 years ago

Thank you very much for your pull request! For receiving the free PineCone, please sign up at this link. (If there will be any issues with signing up, please let me know here).