InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

WparamReturn #4664

Closed andrei-sandor closed 1 month ago

andrei-sandor commented 1 month ago

Contains the following fixes

dzenanz commented 1 month ago

It looks like you need to apply clang-format to some of the changed files.

andrei-sandor commented 1 month ago

It looks like you need to apply clang-format to some of the changed files.

Error: Code is inconsistent with ITK Coding Style. Add the action:ApplyClangFormat PR label to correct.

This is the message generated by clang-format linter. Is it possible to type action:ApplyClangFormat in a comment here? I am working with a different version of clang-format which gives different formatting changes than here.

dzenanz commented 1 month ago

I added that label (in GitHub UI on the right), but the action triggered (Apply clang-format to PR / clang-format (pull_request) in the checks list) crashes. ITK uses clang-format 8.0.3.

I forgot who created clang-format action, @thewtex or someone else (@jhlegarreta?).

@blowekamp has a parallel PR #4651 to update clang-format used to the latest version.

jhlegarreta commented 1 month ago

I forgot who created clang-format action, @thewtex or someone else (@jhlegarreta?).

I don't think it was me. Cannot remember if adding the label or calling something like clang-format as a comment to PRs has ever worked.

dzenanz commented 1 month ago

Now that you mentioned it, I think it used to work for PRs made from branches in this repository, but not from branches in forks. But I think that the action itself might be broken now.

thewtex commented 1 month ago

I created the Apply Clang Format Action. At the time it was created, it was limited in its ability to apply to PR's from forks because of permission functionality in GitHub Actions. Since then, I think GitHub has added the capability to indicate that an Action can push to a PR, etc. If someone wants to look into the adding the config required, please do. I plan to spend time helping with the pre-commit hook / action setup as a replacement.