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.

Sign up for GitHub

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

Jump to bottom

Fix CI errors #317

Merged
merged 5 commits into from
Dec 27, 2021
Merged

Fix CI errors #317

merged 5 commits into from
Dec 27, 2021

Conversation

dzenanz
Copy link
Member

@dzenanz dzenanz commented Dec 27, 2021

I am not sure whether this will fix all errors, but it will surely fix some.

@github-actions github-actions bot added area:Filtering Issues affecting the Filtering module language:C++ Changes to C++ examples type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels Dec 27, 2021
@dzenanz
STYLE: fix lint errors by applying clang-format
84e4187
@dzenanz
ENH: require C++14 for examples
048396a 
First warning and first error (of about 50):

Modules/Core/Common/itkConfigure.h:56:6: warning: #warning "WARNING:  The current project is configured to use a C++ standard version older than the C++ standard used for this build of ITK" [-Wcpp]

Modules/Core/Common/include/itkSize.h:329:29: error: 'constexpr const value_type& itk::Size<VDimension>::operator[](itk::Size<VDimension>::size_type) const' cannot be overloaded
@dzenanz
ENH: use new version of super linter
83c917c
@dzenanz
COMP: fix compile errors by aggressive code cleanup
02d730f 
The errors were introduced by 89a7300.

/home/runner/work/Ex/src/Filtering/LabelMap/ShapeAttributesForBinaryImage/Code.cxx: In function 'int main(int, char**)':
/home/runner/work/Ex/src/Filtering/LabelMap/ShapeAttributesForBinaryImage/Code.cxx:48:16: error: 'argv' was not declared in this scope
     fileName = argv[1];
                ^~~~

../src/Filtering/Thresholding/DemonstrateThresholdAlgorithms/Code.cxx:52:0: error: unterminated #ifdef
@dzenanz
ENH: use CMake version 3.16.3 as new required minimum
ad2b310
@github-actions github-actions bot added area:Bridge Issues affecting the Bridge module area:Core Issues affecting the Core module area:IO Issues affecting the IO module area:Nonunit Issues affecting the Nonunit module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Remotes Issues affecting the Remote module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module labels Dec 27, 2021
@dzenanz dzenanz merged commit 345b1d0 into InsightSoftwareConsortium:master Dec 27, 2021
@dzenanz
Copy link
Member Author

dzenanz commented Dec 27, 2021

After this PR, we are just one warning short of a clean CI:

[1587/1588] Generating sphinx documentation: html
Running Sphinx v3.0.4
WARNING: while setting up extension sphinx.addnodes: node class 'meta' is already registered, its visitors will be overridden

@jhlegarreta
Copy link
Member

After this PR, we are just one warning short of a clean CI:

Awesome @dzenanz !

Looks like the error stems from using docutils 0.18.1 (latest) and a not very recent sphinx (latest is 4.3.2):
sphinx-doc/sphinx#9841 (comment)

The CI's sphinx version is set in:
https://github.com/InsightSoftwareConsortium/ITKSphinxExamples/blob/master/.github/workflows/build-test-publish.yml#L285

I'd say that the sphinx version be updated. @thewtex is there a particular, more recent version that we should adopt?

@dzenanz
Copy link
Member Author

dzenanz commented Dec 27, 2021

Matt is currently out of office, and is generally busy. Maybe we should try 3.5.4 (released Apr 11, 2021), as the latest from 3.x branch? Or the latest, 4.3.2 (released Dec 19, 2021)? I guess that we should go with the latest one which works for us.

@jhlegarreta
Copy link
Member

I asked because maybe the version should match that one used to actually build the site, and I do not know whether it was kept at 3.0.4 for some particular reason. I'm fine if you decide that we should go either with the latest from 3.x or the most recent one.

@dzenanz
Copy link
Member Author

dzenanz commented Dec 27, 2021

I think that Sphinx documentation is built and deployed upon merge. Or at least it was. So any configuration should be in the repository (CI etc).

@dzenanz
Copy link
Member Author

dzenanz commented Dec 27, 2021

Going with 3.x is less likely to cause problems (e.g. new warnings), so I think that is lower hanging fruit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjmjohnson hjmjohnson hjmjohnson approved these changes

@jhlegarreta jhlegarreta Awaiting requested review from jhlegarreta

Assignees
No one assigned
Labels
area:Bridge Issues affecting the Bridge module area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Nonunit Issues affecting the Nonunit module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Remotes Issues affecting the Remote module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module language:C++ Changes to C++ examples type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dzenanz @jhlegarreta @hjmjohnson