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

Bump clang-format and ruff versions to latest #496

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Conversation

JelteF
Copy link
Collaborator

@JelteF JelteF commented Dec 16, 2024

In #98 we started using the same version of clang-format that DuckDB uses. That version is pretty ancient though. So it misses a bunch of features that we might want to use, and also has a bunch of bugs which result in sub-optimal formatting. This updates clang-format to the latest version, and while we're at it we also update ruff.

To be able to reformat all existing files, this also adds a format-all target to our Makefile.

The problem was called out by @dpxcc in #493, but I've been also running into this myself for a while.

@JelteF JelteF force-pushed the bump-formatting-tools branch from 4dc7f70 to 5a9a1bd Compare December 16, 2024 10:17
Copy link
Collaborator

@Y-- Y-- left a comment

Choose a reason for hiding this comment

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

Thanks!

DefineCustomVariable("duckdb.autoload_known_extensions",
"Whether known extensions are allowed to be automatically loaded when a DuckDB query depends on them",
&duckdb_autoload_known_extensions, PGC_SUSET);
DefineCustomVariable(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting that the other in this file weren't changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed, I guess the description was not long enough. Probably we want to tweak the clang-format settings to format these consistently, but let's save that for a follow up PR.

@JelteF JelteF merged commit eb2c6bb into main Dec 16, 2024
5 checks passed
@JelteF JelteF deleted the bump-formatting-tools branch December 16, 2024 12:07
JelteF added a commit that referenced this pull request Dec 20, 2024
In #496 I failed to include the `.hpp` in my clang-format run. This does
that too, and updates our `format-all` rule accordingly.
JelteF added a commit that referenced this pull request Dec 20, 2024
In #496 I failed to include the `.hpp` in my clang-format run. This does
that too, and updates our `format-all` rule accordingly.
JelteF added a commit that referenced this pull request Jan 3, 2025
In #496 we started using the newest clang-format to format our files,
but CI was still installing the old version. This meant that we were not
catching some unformatted files correctly. An example of this being:
#511 (comment)

This starts using the correct clang-format version in CI too and formats
any incorrectly formatted files.
@JelteF JelteF mentioned this pull request Jan 3, 2025
Y-- pushed a commit that referenced this pull request Jan 6, 2025
In #496 we started using the newest clang-format to format our files,
but CI was still installing the old version. This meant that we were not
catching some unformatted files correctly. An example of this being:
#511 (comment)

This starts using the correct clang-format version in CI too and formats
any incorrectly formatted files.
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