Skip to content

Conversation

@tbirdso
Copy link
Contributor

@tbirdso tbirdso commented May 31, 2022

@tbirdso tbirdso requested a review from dzenanz May 31, 2022 15:54
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.

cmake_minimum_required of 3.10.2 is not enough.
CMake Warning at D:/a/ITKPolarTransform/ITK/CMake/ITKModuleExternal.cmake:12 (message):
-- This is needed to allow proper setting of CMAKE_MSVC_RUNTIME_LIBRARY.
-- Do not be surprised if you run into link errors of the style:
LNK2038: mismatch detected for 'RuntimeLibrary': value 'MTd_Static' doesn't match value 'MDd_Dynamic' in module.obj
cmake_minimum_required must be at least 3.16.3
Call Stack (most recent call first):
CMakeLists.txt:7 (include)

@dzenanz
Copy link
Member

dzenanz commented Jun 1, 2022

It looks like further minor maintenance is needed here.

@tbirdso
Copy link
Contributor Author

tbirdso commented Jun 1, 2022

I've seen the failures above before, we should address at some point but they do not seem to typically fail CI. I think the main cause of failures is here:

/work/include/itkCartesianToPolarTransform.hxx:53:44: error: conversion from ‘itk::Vector<double, 2>’ to non-scalar type ‘const InputPointType’ {aka ‘const itk::Point<double, 2>’} requested

@dzenanz
Copy link
Member

dzenanz commented Jun 2, 2022

Currently, this remote is green. I prefer to wait with this PR until we fix the problems which arise from updating ITK.

@dzenanz
Copy link
Member

dzenanz commented Feb 6, 2025

@jhlegarreta This PR is more thorough about replacing http by https. We can probably still use most of the changes proposed here.

@jhlegarreta
Copy link
Member

@jhlegarreta This PR is more thorough about replacing http by https. We can probably still use most of the changes proposed here.

@dzenanz You have just merged PR #30, so feel free to rebase this on main, rework the commit message and force push. I do not have the time to investigate the changes.

@dzenanz dzenanz merged commit 1f6bff4 into main Feb 7, 2025
16 of 55 checks passed
@dzenanz dzenanz deleted the http_to_https branch February 7, 2025 20:57
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.

4 participants