acil-bwh / ChestImagingPlatform

Chest Imaging Platform (CIP)
Other
55 stars 34 forks source link

Fix build against vtk greater 9.1 and itk with custom namespace #46

Closed jcfr closed 1 year ago

jcfr commented 1 year ago

See https://github.com/Slicer/Slicer/issues/6587

jcfr commented 1 year ago

Remaining error to fix:

/home/jcfr/Projects/SlicerCIP-Release/CIP/Utilities/LesionSizingToolkit/itkCannyEdgeDetectionRecursiveGaussianImageFilter.hxx:101:34:
error: ISO C++17 does not allow dynamic exception specifications
  101 | ::GenerateInputRequestedRegion() throw(InvalidRequestedRegionError)
      |                                  ^~~~~
lassoan commented 1 year ago

@jcfr we switched back to using the Slicer fork - see https://github.com/acil-bwh/ChestImagingPlatform/pull/45.

So, you may consider closing this PR here and submit to https://github.com/Slicer/ChestImagingPlatform/ instead.

acil-bwh commented 1 year ago

@jcfr we switched back to using the Slicer fork - see #45.

So, you may consider closing this PR here and submit to https://github.com/Slicer/ChestImagingPlatform/ instead.

Sorry for doing this. I can integrate back the changes on the fork after you are done so we don't block your PRs

acil-bwh commented 1 year ago

PR has been moved to a Slicer fork

lassoan commented 1 year ago

If you are not comfortable with us making changes directly in this repository then using two repositories is the way to go. This allows us to immediately act on urgent issues and then whenever you have time you can fetch updates from our repository.

lassoan commented 1 year ago

I see that you have now granted me write access to this repository, thank you. I guess this means that you are open to accept pull requests from us and OK with us merging them if they are critical and trivial (mostly build script changes). We'll finish this round of updates and testing in our fork and then get back with pull requests here, maybe in a few weeks.

rjosest commented 1 year ago

Hi Andras,

That sounds like a plan. Iā€™m totally conformable with you making changes directly into the repo. But we can follow with fetches from the fork. Happy to hear your thoughts about the best way to do that once you are done with the compatibility changes

Thanks

On Oct 28, 2022, at 9:33 AM, Andras Lasso @.***> wrote:

I see that you have now granted me write access to this repository, thank you. I guess this means that you are open to accept pull requests from us and OK with us merging them if they are critical and trivial (mostly build script changes). We'll finish this round of updates and testing in our fork and then get back with pull requests here, maybe in a few weeks.

ā€” Reply to this email directly, view it on GitHub https://github.com/acil-bwh/ChestImagingPlatform/pull/46#issuecomment-1295006910, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYPZEUBNNH7KE52XK5J3ILWFPI25ANCNFSM6AAAAAAROART2Q. You are receiving this because you are subscribed to this thread.

jcfr commented 1 year ago

Thanks @rjosest , it is great that we can now more easily update the upstream projects šŸ™šŸ‘Œ

Thanks @lassoan for integrating the change on the Slicer fork, I will finalize the update shortly to ensure we can build the project.

jcfr commented 1 year ago

to follow up, the issue related to

error: ISO C++17 does not allow dynamic exception specifications

was already fixed the Slicer/ChestImagingPlatform fork in e80f3738cbedf1a36582bc3a0fa49b11aa6f843a