Skip to content

Conversation

@danmar
Copy link
Owner

@danmar danmar commented Aug 28, 2025

No description provided.

@danmar
Copy link
Owner Author

danmar commented Aug 28, 2025

@jcfr feel free to review

Copy link
Contributor

@jcfr jcfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this:

  1. Considering cherry picking commit titled chore: Remove obsolete runastyle scripts from #516

  2. Add a second commit

    chore: Add settings corresponding to uncrustify 0.80.1
    
    Introduce settings based of similar settings copied from 
    danmar/cppcheck@b4f07cdbb.
    
  3. Add a third commit:

    style: Consistent formatting of C++ sources using uncrustify
    
    This commit updates the sources file based on the settings
    introduced in previous commit.
    
  4. Either in the context of this pull request or in a follow-up one, add .git-blame-ignore-revs1

  5. Add pre-commit.yml workflow with the uncrustify hook enabled2.

Footnotes

  1. https://docs.github.com/en/repositories/working-with-files/using-files/viewing-and-understanding-files#ignore-commits-in-the-blame-view

  2. https://github.com/pocc/pre-commit-hooks

@danmar danmar force-pushed the switch-to-uncrustify branch from 4f106fe to ecf23c1 Compare August 28, 2025 17:40
@danmar danmar force-pushed the switch-to-uncrustify branch from ecf23c1 to 18cfb6a Compare August 28, 2025 17:44
@danmar danmar merged commit cfd1797 into master Aug 28, 2025
26 checks passed
@danmar danmar deleted the switch-to-uncrustify branch August 28, 2025 17:49
@danmar
Copy link
Owner Author

danmar commented Aug 28, 2025

@jcfr thanks for the reviews. there was some mess in my first commit to cleanup as you saw. but I hope it didn't add any extra changes now.

# - you can put uncrustify in your PATH
# - you can create an environment variable UNCRUSTIFY that has the full path of the binary

UNCRUSTIFY_VERSION="0.72.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely incorrect and inconsistent, inspecting danmar/cppcheck@3726ace seems to indicate that the version should be 0.80.1

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I copied the file directly from cppcheck main though. :-( maybe we need to update the script both in cppcheck and simplecpp..

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we need to update the script both in cppcheck and simplecpp..

would you have time to look at that maybe?

@jcfr
Copy link
Contributor

jcfr commented Aug 28, 2025

The commit should probably have been split as suggested in #517 (review). Indeed, referencing 18cfb6a in .git-blame-ignore-revs will now incorrectly ignore non stylistic changes.

While this is not a critical in the current situation, in the future, I suggest to avoid mixing changes.

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.

2 participants