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.

Sign up for GitHub

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

Jump to bottom

Improve clang-format to check newlines at end of file #2127

Open
1 task done
NeroBurner opened this issue Sep 21, 2024 · 4 comments · May be fixed by #2139
Open
1 task done

Improve clang-format to check newlines at end of file #2127

NeroBurner opened this issue Sep 21, 2024 · 4 comments · May be fixed by #2139
Labels
maintenance Background work

Comments

@NeroBurner
Copy link
Contributor

Verification

  • I searched for similar issues (including closed issues) and found none was relevant.

Introduce the issue

I regularly criticize missing newline at end of file resulting in stop symbol of GitHub
stop symbol of GitHub at end of files

@mark9064 suggested to let clang-format do this check #2107 (comment)

@FintasticMan adds info on which clang-format version we could use #2107 (comment)

I've just checked, and the option to insert trailing newlines has been added in clang-format 16. Last time I updated it was 14, so it wasn't included in that. I'll do a upgrade to a newer version soon.

What version do you think is reasonable to update to? Unfortunately, I don't think that clang-format ignores unrecognised options, so it'll be a minimum version for anyone using it.

Preferred solution

clang-format checks the missing newline at end of file in CI

Version

main
@NeroBurner NeroBurner mentioned this issue Sep 21, 2024
Merged
@FintasticMan
Copy link
Member

The latest version of clang-format is 19, the latest version in the Ubuntu 24.04 repositories (which we use for CI) is 18, and the latest version in the Debian stable repos is 16 (just for a more conservative comparison). I'd be quite happy to go straight for 18, but I really don't know which versions other developers are using.

If anyone is using a version of clang-format below 18, could you let me know here, along with which OS and version? Thanks!

@NeroBurner
Copy link
Contributor Author

Ubuntu 22.04 has clang-format-14

https://packages.ubuntu.com/jammy/clang-format

Clang-format-16 is available since Ubuntu 23.10
https://packages.ubuntu.com/search?keywords=clang-format-16

Just a data point, although I'm on arch so I have newer packages available

@mark9064
Copy link
Member

mark9064 commented Oct 5, 2024 •
edited
Loading

In other words, the option we want is supported by Debian stable, Ubuntu LTS and anything newer right? So in that case I think we're safe to enable it

Do we even specify a minimum clang format version anywhere (in code rather than docs)? If so I think it shouldn't be bumped past Debian stable (16) unless we want some newer feature

@FintasticMan
Copy link
Member

We do specify a minimum version in the clang-format commit hook, so I'll just bump to 16 for now.

@FintasticMan FintasticMan linked a pull request Oct 9, 2024 that will close this issue
Open
@FintasticMan FintasticMan linked a pull request Oct 27, 2024 that will close this issue
clang-format: Update to 16 #2139
Open
@mark9064 mark9064 added the maintenance Background work label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
maintenance Background work
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

clang-format: Update to 16 InfiniTimeOrg/InfiniTime
3 participants
@NeroBurner @mark9064 @FintasticMan