ENH: Upgrade CI for ITK 5.4.0 #40
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Assign copyright to the Insight Software Consortium and use the Apache 2 license, ITK's current license. This will be merged pending approval by Glenn Pierce.
This commit updates CMakeLists.txt to ensure built library is shared. It also uses "RegisterFactoryInternal" to make sure the factory is registered once into the ImageIOFactory. See InsightSoftwareConsortium/ITK@39b4ab3 (BUG: Internal factory must use RegisterFactoryInternal method) for more details. See ITK issue #3393 https://issues.itk.org/jira/browse/ITK-3393
This new parameter allows ITK to be aware any object that needs to be registered to the ITK factory. Remote modules that offer IO capabilities can be more easily integrated in ITK.
Require CMake minimum version to be 3.9.5 following ITKv5 requiring C++11: https://discourse.itk.org/t/minimum-cmake-version-update/585
Provide remove virtual and override Use clang-tidy to add ITK_OVERRIDE, and to remove redundant virtual on functions. cd ../ITK; clang-tidy -p ITK-clangtidy $find Modules/[A-J]* -name *.cxx |fgrep -v ThirdParty) -checks=-*,modernize-use-override -header-filter=.* -fix clang-tidy -p ITK-clangtidy $(find Modules/[K-Z]* -name *.cxx |fgrep -v ThirdParty) -checks=-*,modernize-use-override -header-filter=.* -fix https://stackoverflow.com/questions/39932391/virtual-override-or-both-c When you override a function you don't technically need to write either virtual or override. The original base class declaration needs the keyword virtual to mark it as virtual. In the derived class the function is virtual by way of having the ¹same type as the base class function. However, an override can help avoid bugs by producing a compilation error when the intended override isn't technically an override. E.g. that the function type isn't exactly like the base class function. Or that a maintenance of the base class changes that function's type, e.g. adding a defaulted argument. In the same way, a virtual keyword in the derived class can make such a bug more subtle, by ensuring that the function is still is virtual in further derived classes. So the general advice is, virtual for the base class function declaration. This is technically necessary. Use override (only) for a derived class' override. This helps with maintenance.
git grep -l "ITK_OVERRIDE" | fgrep -v itk_compiler_detection.h | fgrep -v CMakeLists.txt |fgrep -v .cmake | xargs sed -i '' -e "s/ITK_OVERRIDE/override/g"
Some headers from C library were deprecated in C++ and are no longer welcome in C++ codebases. Some have no effect in C++. For more details refer to the C++ 14 Standard [depr.c.headers] section. This patch replaces C standard library headers with their C++ alternatives and removes redundant ones.
STYLE: Use auto for variable creation
This check is responsible for using the auto type specifier for variable
declarations to improve code readability and maintainability.
The auto type specifier will only be introduced in situations where the
variable type matches the type of the initializer expression. In other words
auto should deduce the same type that was originally spelled in the source
cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-use-auto -header-filter=.* -fix
use auto when declaring iterators
use auto when initializing with a cast to avoid duplicating the type name
use auto when initializing with a template cast to avoid duplicating the type name
use auto when initializing with new to avoid duplicating the type name
SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD
PERF: Replace explicit return calls of constructor
Replaces explicit calls to the constructor in a return with a braced
initializer list. This way the return type is not needlessly duplicated in the
function definition and the return statement.
SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD
cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-return-braced-init-list -header-filter=.* -fix
PERF: Allow compiler to choose best way to construct a copy
With move semantics added to the language and the standard library updated with
move constructors added for many types it is now interesting to take an
argument directly by value, instead of by const-reference, and then copy. This
check allows the compiler to take care of choosing the best way to construct
the copy.
The transformation is usually beneficial when the calling code passes an rvalue
and assumes the move construction is a cheap operation. This short example
illustrates how the construction of the value happens:
class Foo {
public:
- Foo(const std::string &Copied, const std::string &ReadOnly)
- : Copied(Copied), ReadOnly(ReadOnly) {}
+ Foo(std::string Moved, const std::string &ReadOnly)
+ : Copied(std::move(Moved)), ReadOnly(ReadOnly) {}
private:
private:
std::string Copied;
const std::string &ReadOnly;
};
SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD
cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-pass-by-value -header-filter=.* -fix
STYLE: Use range-based loops from C++11
Used as a more readable equivalent to the traditional for loop operating
over a range of values, such as all elements in a container, in the
forward direction.
====
Range based loopes are more explicit for only computing the
end location once for containers.
for ( ImageIORegion::IndexType::const_iterator i = this->GetIndex().begin();
i != this->GetIndex().end(); //<- NOTE: Compute end every loop iteration
++i )
for (long i : this->GetIndex()) //<- NOTE: Implicitly only compute end once
====
Explicitly reduce the amount of index computations: (The compiler
probably does this too)
for(int i = 0; i < 11; i++)
{
pos[0] = testPoints[i][0];
pos[1] = testPoints[i][1];
^^^^
for(auto & testPoint : testPoints)
{
pos[0] = testPoint[0];
pos[1] = testPoint[1];
====
SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD
cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-loop-convert -header-filter=.* -fix
== http://en.cppreference.com/w/cpp/language/type_alias == Type alias is a name that refers to a previously defined type (similar to typedef). A type alias declaration introduces a name which can be used as a synonym for the type denoted by type-id. It does not introduce a new type and it cannot change the meaning of an existing type name. There is no difference between a type alias declaration and typedef declaration. This declaration may appear in block scope, class scope, or namespace scope. == https://www.quora.com/Is-using-typedef-in-C++-considered-a-bad-practice == While typedef is still available for backward compatibility, the new Type Alias syntax 'using Alias = ExistingLongName;' is more consistent with the flow of C++ than the old typedef syntax 'typedef ExistingLongName Alias;', and it also works for templates (Type alias, alias template (since C++11)), so leftover 'typedef' aliases will differ in style from any alias templates.
Use constexpr for constant numeric literals.
Move `ITK_DISALLOW_COPY_AND_ASSIGN` calls to public section following the discussion in https://discourse.itk.org/t/noncopyable If legacy (pre-macro) copy and assing methods existed, subsitute them for the `ITK_DISALLOW_COPY_AND_ASSIGN` macro.
As agreed in: https://discourse.itk.org/t/cmake-update/870/ Set the `cmake_minimum_required` to version **3.10.2**.
As discussed in: http://review.source.kitware.com/#/c/12655/ the use of the template keyword "class" was substituted by "typename" in the toolkit for the reasons stated in that topic.
Clean the temporary, accidentally comitted KW style file `ITKKWStyleOverwrite.txt` file.
Add CI for the module: - Add Circle, Travis and AppVeyor CI configuration `*.yml` files. - Add the `CTestConfig.cmake` file to submit builds to the **Inisght** project in the **open.cdash.org** dashboard. - Add the Python setup `setup.py` file. - Re-use the `README` file documentation in the `itk-module.cmake` file.
Add the `LICENSE` license file, in conformance to the license notice in the header of the source files.
Improve the `README` file: - Use resTructuredText syntax to conform to `ITKModuleTemplate` guidelines. - Improve its contents. - Add the CI status badges.
Improve Python package information: - Make the reference in the long description match the package name and not the ITK project name. - Use white spaces to avoid words in two consecutive lines being concatenated. Partially addresses: InsightSoftwareConsortium/ITK#326
Fix CMake configuration warning. Fixes: ``` Make Warning (dev) at C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:102 (message): Unknown argument [ the code was brought up to current ITK standards by `Nicholas Tustison <mailto:mtustison@gmail.com>`_] Call Stack (most recent call first): itk-module.cmake:12 (itk_module) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleExternal.cmake:92 (include) CMakeLists.txt:8 (include) This warning is for project developers.Use -Wno-dev to suppress it. CMake Warning (dev) at C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:102 (message): Unknown argument [ and it was converted to an ITK Remote module by `Kent Williams <mailto:norman-k-williams@uiowa.edu>`_. ] Call Stack (most recent call first): itk-module.cmake:12 (itk_module) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleExternal.cmake:92 (include) CMakeLists.txt:8 (include) This warning is for project developers.Use -Wno-dev to suppress it. CMake Warning (dev) at C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:102 (message): Unknown argument [ the code was brought up to current ITK standards by `Nicholas Tustison <mailto:mtustison@gmail.com>`_] Call Stack (most recent call first): itk-module.cmake:12 (itk_module) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:134 (include) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleExternal.cmake:102 (itk_module_impl) CMakeLists.txt:8 (include) This warning is for project developers.Use -Wno-dev to suppress it. CMake Warning (dev) at C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:102 (message): Unknown argument [ and it was converted to an ITK Remote module by `Kent Williams <mailto:norman-k-williams@uiowa.edu>`_. ] Call Stack (most recent call first): itk-module.cmake:12 (itk_module) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:134 (include) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleExternal.cmake:102 (itk_module_impl) CMakeLists.txt:8 (include) This warning is for project developers.Use -Wno-dev to suppress it. CMake Warning (dev) at C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:102 (message): Unknown argument [ the code was brought up to current ITK standards by `Nicholas Tustison <mailto:mtustison@gmail.com>`_] Call Stack (most recent call first): itk-module.cmake:12 (itk_module) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:325 (include) test/CMakeLists.txt:1 (itk_module_test) This warning is for project developers.Use -Wno-dev to suppress it. CMake Warning (dev) at C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:102 (message): Unknown argument [ and it was converted to an ITK Remote module by `Kent Williams <mailto:norman-k-williams@uiowa.edu>`_. ] Call Stack (most recent call first): itk-module.cmake:12 (itk_module) C:/SDKs/ITK/itk-head/ITK/CMake/ITKModuleMacros.cmake:325 (include) test/CMakeLists.txt:1 (itk_module_test) This warning is for project developers.Use -Wno-dev to suppress it. ``` Take advantage of the commit to fix Nick's email address.
Fix `itk-module.cmake` local variable typo: change `MY_CURENT_DIR` to `MY_CURRENT_DIR`.
Conform to ITK module naming convetion: change module name from `ITKFDFImageIO` to `ITKIOFDF`. Closes InsightSoftwareConsortium#24.
```
git filter-branch -f \
--tree-filter "~/ITK/Utilities/Maintenance/clang-format.bash --clang-format ~/Dashboard/src/ITK-clang11/clang-format-Linux --tracked" \
master..
```
Find and remove redundant void argument lists.
Finds and replaces integer literals which are cast to bool. SRCDIR= #My local SRC BLDDIR= #My local BLD cd run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,modernize-use-bool-literals -header-filter=.* -fix
This check replaces default bodies of special member functions with = default;. The explicitly defaulted function declarations enable more opportunities in optimization, because the compiler might treat explicitly defaulted functions as trivial. Additionally, the C++11 use of = default more clearly expreses the intent for the special member functions.
Find and remove redundant void argument lists.
The emptiness of a container should be checked using the empty() method instead of the size() method. It is not guaranteed that size() is a constant-time function, and it is generally more efficient and also shows clearer intent to use empty(). Furthermore some containers may implement the empty() method but not implement the size() method. Using empty() whenever possible makes it easier to switch to another container in the future. SRCDIR= #My local SRC BLDDIR= #My local BLD cd run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,readability-container-size-empty -header-filter=.* -fix
The mission of NumFOCUS is to promote open practices in research, data, and scientific computing. https://numfocus.org
…the incorporation of 2 yml scripts that will test the module under various configurations for Windows, Linux and MacOS. These scripts were copied from the ITKModuleTemplate.
Fixes changes made in #2053. ITK_DISALLOW_COPY_AND_ASSIGN will be used if ITK_FUTURE_LEGACY_REMOVE=OFF.
As the GitHub Actions have been integrated, there is no need for the previous CI for this module. This specifically removes CircleCI, Appveyor and Travis CI.
Removal of .h includes from same named .hxx files
requires removal of outdated rule that is no longer
relevant.
This PR will update the version of ITK that is
used for the remove modules."
BRANCH_NAME=update-reference-itk-version
for remdir_script in *.remote.cmake; do
remdir="${remdir_script//*.remote.cmake/}"
if [ ! -d "${remdir}" ] ;then
echo "Missing directory ${remdir}"
continue
fi
pushd "${remdir}" || exit
echo "=============== $(pwd) ========="
git checkout master
git fetch origin
git rebase origin/master
git checkout -b ${BRANCH_NAME}
sed 's/itk-git-tag : .*/itk-git-tag: "a89145bccda6a36f42cfdd45d3a6b27234ff54fe"/g' $( fgrep -Rl "itk-git-tag:" |fgrep yml )
git status
diff_line_no=$(git diff |wc -l )
if [ "${diff_line_no}" -ne 0 ]; then
git add -p
git commit -F /tmp/update_itk_msg
gh pr create -a "@me" -F /tmp/update_itk_msg
else
echo "Skipping changes $(pwd)"
fi
git fetch
git rebase origin/master
git push origin -f ${BRANCH_NAME}:${BRANCH_NAME}
popd || exit
done
Upgrade CI testing to use the latest ITK 5.4.0 version and CI configuration. Committed via https://github.com/asottile/all-repos
thewtex
force-pushed
the
all-repos_autofix_ci-itk-5.4.0-secret-syntax
branch
from
2e8590c
to
d8a0b69
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upgrade CI testing to use the latest ITK 5.4.0 version and CI configuration.
Committed via https://github.com/asottile/all-repos