Add format checker to the CI

[Dtenwolde]

This PR adds a more extensive formatting checker for extensions and a format check step to the CI.

I copied the format.py script from https://github.com/duckdb/duckdb/blob/main/scripts/format.py and renamed it to format_extension.py since it has different directories to check. I also added similar Makerules such as format-check and format-fix. I think keeping the pipeline consistent with the main duckdb repo makes sense for the code quality of extensions. Extension maintainers can always opt out or disable the workflow if they so desire.

Two things I am not sure about:

• The current format script works for C++, Python, Java and .test files. What do you think about extensions written in Rust?
• There is code to check the header of .hpp files (see below for the strings), but can we enforce that extensions have such a header? I am not sure how strict we should be about this check or if it is even relevant. Curious about your thoughts.

header_top = "//===----------------------------------------------------------------------===//\n"
header_top += "//                         DuckDB\n" + "//\n"
header_bottom = "//\n" + "//\n"
header_bottom += "//===----------------------------------------------------------------------===//\n\n"

I added a PR to the extension-template to trigger this workflow: duckdb/extension-template#99

[carlopi]

Hi, thanks a lot!

I have not take a look at the code yet, one feedback on the setup we have is that https://github.com/duckdb/extension-ci-tools/blob/main/.github/workflows/TestCITools.yml should likely invoke the formatting workflow, so that directly on this PR that is invoked, and on changes to the formatting script this is still run once so that it's checked to still make sense (where make sense means not exploding).

Note that it's possible that adding the test means it would fail, since extension-template might not be formatted properly yet, but that's OK, relevant thing is checking it completes without burning.

[dentiny]

curious, why L137 uses python while L140 and L143 uses python3?

[Dtenwolde]

Fair point, using python for all in cff0722 (#120)

[dentiny]

Maybe I'm too sensitive, I'm not sure whether MIT license should be included
From doc, the copyright notice needs necessary.
(Though I know these two repositories belong to the same organization...)

[Dtenwolde]

I wouldn't be opposed to adding this, but the other scripts in this repo also don't have this in the header. So I think this is up to @carlopi whether this should be added or not.

[dentiny]

nit: newliner

[Dtenwolde]

Fix: Dtenwolde@eb48b6e

[dentiny]

Thank you so much!

[carlopi]

I think you should add a step like:

      - uses: actions/checkout@v4
        name: Checkout Extension CI tools
        with:
          path: 'extension-ci-tools'
          ref: ${{ inputs.ci_tools_version }}
          repository: ${{ inputs.override_ci_tools_repository }}

and add ci_tools_version AND override_ci_tools to the input, AND pass them down on the test invocation so that they are the current branch / current repo.

You can look at how https://github.com/duckdb/extension-ci-tools/blob/main/.github/workflows/_extension_distribution.yml works, or I can take over later.