Skip to content

Conversation

@hjmjohnson
Copy link
Member

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
Member Author

@dzenanz Part of slicer updates

@hjmjohnson
Copy link
Member Author

@dzenanz Updates for supporting newer ITK.

@dzenanz
Copy link
Member

dzenanz commented Jan 27, 2025

PR #85 maybe needs to be done first.

@hjmjohnson hjmjohnson force-pushed the fix-legacy-remove branch 3 times, most recently from 8f286c1 to 151c40e Compare January 28, 2025 02:59
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 guess we will have to leave CI greening for some later date. With these changes, does this build when Module_IOScanco is turned on?

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
The convention of only specifying the MAJOR version
is the indicator that the latest version in that
series should be used.

By not specifying the MINOR and PATCH, the exact versions
is not pinned, but the latest in that series is chosen.
(i.e. the v5 tag is updated every time a new MINOR or PATCH
tag is generated).

This allows benefiting from minor patch fixes without needing
to update workflows.
Set the default build package tags to v5.4.2
for capturing the ITKRemoteModuleBuildTestPackageAction
shared scripts.

This pulls the default configuration items needed
to build against ITK version v5.4.2.
Match version for ITK v5.4.2
@dzenanz
Copy link
Member

dzenanz commented Mar 17, 2025

Warning: writing 1 or more bytes into a region of size 0 overflows the destination seems like we should take care of it. Reviewing now.

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 that the changes here only expose pre-existing issue with that warning. Still, it is preferable to fix it to have a clean CI.

@dzenanz
Copy link
Member

dzenanz commented May 26, 2025

This should probably be merged before #87. Merge as-is or try to fix the failure?

@hjmjohnson
Copy link
Member Author

@dzenanz I agree with you. I think this should be merged as is. No new errors are represented. I do not have access rights to do the merge.

@dzenanz dzenanz merged commit 01144b9 into InsightSoftwareConsortium:master May 27, 2025
18 of 25 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