Skip to content

Add code quality workflows: format and tidy #74

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Add code quality workflows: format and tidy #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0677ac1
Add code_quality section, both to CI and Makefile
carlopi Jul 2, 2025
3c2f58b
Explicit which checks are performed
carlopi Jul 3, 2025
7906808
Format fixes
carlopi Jul 2, 2025
ef0f1b8
Merge remote-tracking branch 'origin/main' into code_quality
carlopi Jul 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/MainDistributionPipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,13 @@ jobs:
duckdb_version: v1.3-ossivalis
ci_tools_version: main
deploy_latest: ${{ startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main' }}

code-quality:
name: Code Quality
uses: duckdb/extension-ci-tools/.github/workflows/_extension_code_quality.yml@main
with:
extension_name: httpfs
duckdb_version: v1.3-ossivalis
ci_tools_version: main
extra_toolchains: 'python3'
format_checks: 'format;tidy'
20 changes: 20 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,23 @@ CORE_EXTENSIONS=''

# Include the Makefile from extension-ci-tools
include extension-ci-tools/makefiles/duckdb_extension.Makefile

#### Misc
format-check:
python3 duckdb/scripts/format.py --all --check --directories extension/httpfs test
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can just move the extension code to src like other extensions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, moving to src is right, I will do so post 1.3.2 given there are a bunch of in-flight PRs.

Also format code might need to take the paths from a Makefile variable instead of hardcoding, but that's minor.


format:
python3 duckdb/scripts/format.py --all --fix --noconfirm --directories extension/httpfs test

format-fix:
python3 duckdb/scripts/format.py --all --fix --noconfirm --directories extension/httpfs test

format-main:
python3 duckdb/scripts/format.py main --fix --noconfirm --directories extension/httpfs test

tidy-check:
mkdir -p ./build/tidy
cmake $(GENERATOR) $(BUILD_FLAGS) $(EXT_DEBUG_FLAGS) -DDISABLE_UNITY=1 -DCLANG_TIDY=1 -S $(DUCKDB_SRCDIR) -B build/tidy
cp duckdb/.clang-tidy build/tidy/.clang-tidy
cd build/tidy && python3 ../../duckdb/scripts/run-clang-tidy.py '$(PROJ_DIR)extension/httpfs/' -header-filter '$(PROJ_DIR)extension/httpfs/' -quiet ${TIDY_THREAD_PARAMETER} ${TIDY_BINARY_PARAMETER} ${TIDY_PERFORM_CHECKS}

3 changes: 2 additions & 1 deletion extension/httpfs/httpfs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ void TimestampToTimeT(timestamp_t timestamp, time_t &result) {
HTTPFileHandle::HTTPFileHandle(FileSystem &fs, const OpenFileInfo &file, FileOpenFlags flags,
unique_ptr<HTTPParams> params_p)
: FileHandle(fs, file.path, flags), params(std::move(params_p)), http_params(params->Cast<HTTPFSParams>()),
flags(flags), length(0), force_full_download(false), buffer_available(0), buffer_idx(0), file_offset(0), buffer_start(0), buffer_end(0) {
flags(flags), length(0), force_full_download(false), buffer_available(0), buffer_idx(0), file_offset(0),
buffer_start(0), buffer_end(0) {
// check if the handle has extended properties that can be set directly in the handle
// if we have these properties we don't need to do a head request to obtain them later
if (file.extended_info) {
Expand Down
2 changes: 1 addition & 1 deletion extension/httpfs/httpfs_client_wasm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ unique_ptr<HTTPClient> HTTPFSUtil::InitializeClient(HTTPParams &http_params, con

unordered_map<string, string> HTTPFSUtil::ParseGetParameters(const string &text) {
unordered_map<string, string> result;
//TODO: HTTPFSUtil::ParseGetParameters is currently not implemented
// TODO: HTTPFSUtil::ParseGetParameters is currently not implemented
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion test/sql/secret/secret_aws.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# name: test/sql/secret/secret_aws.test
# description: Tests secret refreshing
# group: [secrets]
# group: [secret]

require-env S3_TEST_SERVER_AVAILABLE 1

Expand Down
2 changes: 1 addition & 1 deletion test/sql/secret/secret_refresh.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# name: test/sql/secret/secret_refresh.test
# description: Tests secret refreshing
# group: [secrets]
# group: [secret]

require-env S3_TEST_SERVER_AVAILABLE 1

Expand Down
2 changes: 1 addition & 1 deletion test/sql/secret/secret_refresh_attach.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# name: test/sql/secret/secret_refresh_attach.test
# description: Tests secret refreshing
# group: [secrets]
# group: [secret]

require-env S3_TEST_SERVER_AVAILABLE 1

Expand Down
2 changes: 1 addition & 1 deletion test/sql/secret/test_secret_type.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# name: test/sql/secret/test_secret_type.test
# description: Test the secret types added by this extension
# group: [httpfs]
# group: [secret]

require httpfs

Expand Down
Loading