search

pine64 / bl_iot_sdk

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

Fix code formatting in bl602_hal #63

Closed Ferenc- closed 3 years ago

Ferenc- commented 3 years ago
Avamander commented 3 years ago

This is a great idea, consistency is good, but it will cause conflicts with upstream that at this point would not be very ideal. @gamelaster what do you think?

I will take an another look soon, but maybe the autoformat rules could be changed further to avoid some changes?

Ferenc- commented 3 years ago

Would it make sense the send the PR against upstream instead?

Avamander commented 3 years ago

If you get it upstream, I'll pull it here.

robertlipe commented 3 years ago

Whitespace through the whole tree is a mess. If you can get upstream to remove all the trailing spaces and fix the crazy mixing of tabs and spaces, this won't be a maintenance nightmare. Please go for it. Need help or do you think one monster patch is the way to go?

gamelaster commented 3 years ago

@Ferenc- looks like this get merged to upstream. Can I merge it ?

robertlipe commented 3 years ago

I just opened three upstream PRs which are a huge superset of these. They may go over like a rock, but we'll see. It's currently impossible to blend code to the current style because the current style is a mess of tabs and whitespace and random trailing whitespace and such. I thus didn't change style as much as I made it consistent. @gamelaster , please follow along and please prepare to consider merging (and upvoting):

https://github.com/bouffalolab/bl_iot_sdk/pull/14 https://github.com/bouffalolab/bl_iot_sdk/pull/13 https://github.com/bouffalolab/bl_iot_sdk/pull/12

3rdparty and toolchain are also messy, but I assume they are generated or have upstream upstreams, so I skipped them.

I can mechanically rip through the pine64 tree, too, but wanted to offer them in upstream, too. It's just a big turn-off for contributors to see messy whitespace and inconsistency because it's hard to blend style where then isn't any. :-)

gamelaster commented 3 years ago

Hi @robertlipe , I seen your pull requests, good work! It's good that you pushed it into upstream, I will prepare for it and merge it with upstream. Also, I plan to cherrypick various commits and send them to upstream, but there is no eta :).

robertlipe commented 3 years ago

@gamelaster , @Avamander , @Ferenc- : good news! Upstream was more receptive to this than I anticipated. They requested that I show my work (not unreasonable) and integrated all three of the above PRs. If you'd prefer to pull from upstream, great. If you'd rather me prepare another version and submit separately for your tree, I can do that.

You may or may not find the script in https://github.com/bouffalolab/bl_iot_sdk/pull/12#issuecomment-724160413 useful.

Sublime users should use: https://github.com/bubenkoff/ExpandTabsOnSave-SublimeText and https://blog.revathskumar.com/2012/08/sublimetext-remove-tailing-spaces-on-file-save.html

Maybe someday we can work toward having a presubmit that just deals with it on the pull request: https://github.com/doublify/pre-commit-clang-format

I can help work out a ClangFormat specifiation that matches the majority of the code now in the tree if there's interest.

gamelaster commented 3 years ago

@robertlipe I will merge it from the upstream :) once again thank you for your effort 😊

gamelaster commented 3 years ago

@Ferenc- please, did you received an invitation for free PineCone?