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

WparamReturn #4664

Merged
merged 5 commits into from
May 29, 2024
Merged

Conversation

andrei-sandor
Copy link
Contributor

Contains the following fixes

  • Remove parameters in documentation that are not in the class
  • Remove \return when no return statement
  • Remove \param when no parameter
  • Fix small typo

@github-actions github-actions bot added the area:Core Issues affecting the Core module label May 15, 2024
@dzenanz dzenanz added the action:ApplyClangFormat Add this label to a pull request to apply `clang-format` to the branch label May 15, 2024
@dzenanz
Copy link
Member

dzenanz commented May 15, 2024

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

@andrei-sandor
Copy link
Contributor Author

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
Copy link
Member

dzenanz commented May 15, 2024

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
Copy link
Member

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
Copy link
Member

dzenanz commented May 15, 2024

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
Copy link
Member

thewtex commented May 22, 2024

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.

@thewtex thewtex merged commit 29ea063 into InsightSoftwareConsortium:master May 29, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:ApplyClangFormat Add this label to a pull request to apply `clang-format` to the branch area:Core Issues affecting the Core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants