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

Uncrustify second attempt #843

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

koeppea
Copy link
Member

@koeppea koeppea commented Oct 21, 2017

Since I've accidently closed the PR #516 from @LocutusOfBorg,
I've made up a new on based on his PR.
Of course if fine, I'm resetting the last commit, rebase over current master and reapply the uncrustify changes again.

I've adjusted some of the settings of the uncrustify.cfg.

@sgeto
Copy link
Contributor

sgeto commented May 14, 2018

I still think we should use clang-format instead of Uncrustify for the following reasons:

  • it's more common. The most popular open-source projects use it.

  • it's easy to configure.

  • it doesn't introduce any additional dependencies. We already provide extensive support for clang/LLVM. This means every clang user already has clang-format installed. I'm also about to add clang-tidy support so these could go hand in hand.

@kholia is also in favor of clang-format. Is it possible to set up another poll?

@eaescob
Copy link
Contributor

eaescob commented Jun 27, 2018

How much effort will it be to switch to clang-format? I would hate for @koeppea 's work to go to waste

@koeppea
Copy link
Member Author

koeppea commented Jun 27, 2018

Well most of the work is done by the uncrustify script.
However, uncrustify and cmake-lint have a different philosophy IMHO.
While cmake-lint seem to be a kind of a policy checker during Travis build, checked whenever a new pull request is handed in, uncrustify is just run from time to time to just fix the C-code formatting.

It's more a question of what is more convenient for us?

I'm fine with both approaches, however I guess to make our current code compliant to the CMake rules could be quite heavy from the effort's point of view. I'm not aware if cmake-lint can also be used to correct the format - maybe @sgeto?

The good thing about uncrustify is that it does most work for us.
The good thing about cmake-lint is that it'll keep future commits clean.
[Edit]
Just checked - uncrustify can also used to check the syntax. So can also be used for Travis checks.
I can adjust the uncrustify config to match the CMake formatting rules as close as possible.
[/Edit]

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

3 participants