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

Update clang-format to 18 #2039

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft

Update clang-format to 18 #2039

wants to merge 3 commits into from

Conversation

kounelisagis
Copy link
Member

@kounelisagis kounelisagis commented Aug 16, 2024

Currently, clang-format runs only on linux in a non-standard way:

- name: Check formatting (linux only)'
run: |
source $GITHUB_WORKSPACE/ci/scripts/check_formatting_linux.sh
shell: bash
if: ${{ runner.os == 'Linux' }}

It should be better to use the pypi edition of it: https://pypi.org/project/clang-format/ which also provides pre-commit hook and it's system agnostic.


[sc-43783]

@kounelisagis kounelisagis changed the title Update clang-format to 17 Update clang-format to 18 Aug 16, 2024
@johnkerl
Copy link
Contributor

@kounelisagis @ihnorton can we perhaps have a discussion about syncing clang-format version changes across packages?

For those of us doing development on a single repo, whatever version the repo is using, the developer can easily switch to.

For those of us doing multi-repo development -- and given that clang-format 14 disagrees with clang-format 18 -- we could have a situation where a given developer needs to switch clang-format versions back & forth when pushing PRs to different packages.

I'm aware of personally:

  • Core
  • TileDB-Py
  • TileDB-SOMA
  • TileDB-VCF
  • and perhaps others

I'm all in favor of moving "into the future" -- just flagging the opportunity for a synchronize (unless there is such underway and I've missed it).

@kounelisagis kounelisagis marked this pull request as draft August 18, 2024 00:40
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