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

Fix code formatting in bl602_hal #63

Closed
wants to merge 1 commit into from

Conversation

Ferenc-
Copy link

@Ferenc- Ferenc- commented Nov 2, 2020

  • Fix inconsistent indentations
  • Remove trailing whitespaces
  • Eliminate space and tab mixed indentations

* Fix inconsistent indentations
* Remove trailing whitespaces
* Eliminate space and tab mixed indentations
@Avamander
Copy link

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-
Copy link
Author

Ferenc- commented Nov 2, 2020

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

@Avamander
Copy link

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

@robertlipe
Copy link

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
Copy link
Member

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

@robertlipe
Copy link

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):

bouffalolab#14
bouffalolab#13
bouffalolab#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
Copy link
Member

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
Copy link

@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 bouffalolab#12 (comment) 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
Copy link
Member

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

@Ferenc- Ferenc- closed this Nov 17, 2020
@gamelaster
Copy link
Member

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

tchebb pushed a commit to tchebb/bl_iot_sdk that referenced this pull request Jan 15, 2023
bouffalolab_release_bl_iot_sdk_1.6.36
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.

None yet

4 participants