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

COMP: Use modern macro for name of class #69

Merged
merged 5 commits into from
Jan 27, 2025

Conversation

hjmjohnson
Copy link
Contributor

When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

The future preferred macro should be used
│ - itkTypeMacro
│ + itkOverrideGetNameOfClassMacro

When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

The future preferred macro should be used
│ -  itkTypeMacro
│ +  itkOverrideGetNameOfClassMacro
@hjmjohnson
Copy link
Contributor Author

@dzenanz Part of slicer update preparations.

1 similar comment
@hjmjohnson
Copy link
Contributor Author

@dzenanz Part of slicer update preparations.

Deprecated `itkTypeMacro` and `itkTypeMacroNoParent`, in favor of
`itkOverrideGetNameOfClassMacro` and `itkVirtualGetNameOfClassMacro`, respectively.
@dzenanz
Copy link
Member

dzenanz commented Jan 25, 2025

It looks like you need to apply clang-format.

@hjmjohnson
Copy link
Contributor Author

Should the new clang format be applied to Remote modules?

@hjmjohnson
Copy link
Contributor Author

@dzenanz FYI: I upgraded to match the tooling of ITK.

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

I think we need to bump this:


from 5.4 to master to make CI green.

@dzenanz
Copy link
Member

dzenanz commented Jan 27, 2025

Perhaps after merging InsightSoftwareConsortium/ITKClangFormatLinterAction#7.

@dzenanz
Copy link
Member

dzenanz commented Jan 27, 2025

we need to bump this

I tried to do this here, but there are errors:

#7 [3/7] RUN apt-get update && apt-get install -y   git   wget   && apt-get clean
  #7 CACHED
  
  #8 [4/7] RUN apt-get update && wget https://apt.llvm.org/llvm.sh     && chmod +x llvm.sh     && sudo ./llvm.sh 19     && apt-get install clang-format-19     && apt-get clean
  #8 0.314 Hit:1 http://archive.ubuntu.com/ubuntu noble InRelease
 ...
  #8 1.140 2025-01-27 17:18:44 (39.9 MB/s) - 'llvm.sh' saved [5819/5819]
  #8 1.140 
  #8 1.142 /bin/sh: 1: sudo: not found
  #8 ERROR: process "/bin/sh -c apt-get update && wget https://apt.llvm.org/llvm.sh     && chmod +x llvm.sh     && sudo ./llvm.sh 19     && apt-get install clang-format-19     && apt-get clean" did not complete successfully: exit code: 127

@dzenanz dzenanz merged commit 0209577 into KitwareMedical:main Jan 27, 2025
27 checks passed
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