Skip to content

ENH: Update coding style guidelines#170

Merged
dzenanz merged 8 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:UpdateCodingStyleGuidelines
Dec 14, 2021
Merged

ENH: Update coding style guidelines#170
dzenanz merged 8 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:UpdateCodingStyleGuidelines

Conversation

@jhlegarreta
Copy link
Copy Markdown
Member

@jhlegarreta jhlegarreta commented Dec 10, 2021 •
edited
Loading

  • DOC: Remove uses of itk::ImageTraits
  • DOC: Make test ending messages consistent
  • DOC: Override destructor and prefer = default for trivial bodies
  • DOC: Match the current function deletion practice in ITK
  • DOC: Prefer using pre-increment over post-increment
  • DOC: Update the ITK code style to current automatically enforced
  • DOC: Document brace and brace list initialization preference
  • DOC: Update itk::N4BiasFieldCorrectionImageFilter::Sharpen definition

Fixes #133.

@jhlegarreta
DOC: Remove uses of itk::ImageTraits
67a88b4 
Remove uses of `itk::ImageTraits`.

Cros-referencing related ITK commits:
InsightSoftwareConsortium/ITK@dc9630a
InsightSoftwareConsortium/ITK@d775b4d

Update the template parameters of `itk::Image` to the current
implementation:
https://github.com/InsightSoftwareConsortium/ITK/tree/217c0d408487d454cf9b0f945ab5575c2322ee7a
@jhlegarreta
DOC: Make test ending messages consistent
42d950b 
Make test ending messages consistent.
@jhlegarreta
DOC: Override destructor and prefer = default for trivial bodies
60e70f5 
Mark class destructor with `override` and replace its default body with
= default; to match current practice in ITK.

Cross-referencing related ITK commits, e.g.:
InsightSoftwareConsortium/ITK@7ef2496
InsightSoftwareConsortium/ITK@9b74640
@jhlegarreta
DOC: Match the current function deletion practice in ITK
ce05540 
Match the current function deletion practice in ITK: add the
`ITK_DISALLOW_COPY_AND_ASSIGN` macro to the class' public section.

Cross-referencing the ITK commits:
InsightSoftwareConsortium/ITK@939cee4
InsightSoftwareConsortium/ITK@b0f183b
@jhlegarreta
DOC: Prefer using pre-increment over post-increment
747ffae 
Prefer using pre-increment over post-increment `for` loop local
counters to match current ITK practice.
@github-actions github-actions bot added area:Appendices Issues affecting the Appendices part language:LaTeX Changes to LaTeX code type:BookStyle Changes to book style files type:Enhancement Improvement of existing methods or implementation labels Dec 10, 2021
@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Dec 10, 2021

A few notes:

Pinging @N-Dekker on this.

But maybe I missed them. Anyone is welcome to add a commit to fix any.

  • If other oversights are found, anyone is welcome to rebase interactively, ammend and push force.

  • Maybe better tools to keep this in sync with the ITK code base changes should be found, and ways to enforce these in the ITK code base. e.g. @hjmjohnson would clang be able to reformat these files cleanly next time?

Cross-referencing:
InsightSoftwareConsortium/ITK#1195

@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Dec 11, 2021

Have not found examples liable to be changed to use trailing return types: https://github.com/InsightSoftwareConsortium/ITKSoftwareGuide/pull/162/files

But maybe I missed them. Anyone is welcome to add a commit to fix any.

The SharpenImage example code could be restyled, using trailing return types:

ITKSoftwareGuide/SoftwareGuide/Latex/Appendices/CodingStyleGuide.tex

Lines 2882 to 2885 in 7db4436

typename
N4BiasFieldCorrectionImageFilter< TInputImage, TMaskImage, TOutputImage >::RealImagePointer
N4BiasFieldCorrectionImageFilter< TInputImage, TMaskImage, TOutputImage >
::SharpenImage( const RealImageType *unsharpenedImage ) const

@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Dec 11, 2021 •
edited
Loading

The SharpenImage example code could be restyled, using trailing return types:

Thanks. The definition was outdated. I updated it in ec15294. The new one does not have any return type.

dzenanz
dzenanz reviewed Dec 13, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Great effort Jon!

@jhlegarreta jhlegarreta force-pushed the UpdateCodingStyleGuidelines branch from ec15294 to b53226d Compare December 13, 2021 21:43
dzenanz
dzenanz reviewed Dec 13, 2021
View reviewed changes
@jhlegarreta
DOC: Update the ITK code style to current automatically enforced
7b34899 
Update the ITK code style to the current, automatically enforced style:
- Introduce the use of the `clang-format` style enforcing tool.
- Modify the related passages.
- Update the verbatim code snippets to reflect the new style.

Cross-referencing the ITK change commits:
https://github.com/InsightSoftwareConsortium/ITK/pull/1191/commits
@jhlegarreta jhlegarreta force-pushed the UpdateCodingStyleGuidelines branch from b53226d to 5f6ae91 Compare December 13, 2021 22:21
dzenanz
dzenanz reviewed Dec 13, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Sorry to nitpick again :(

@jhlegarreta
DOC: Document brace and brace list initialization preference
f6f73fe 
Document brace and brace list initialization preference in member
declaration.

Change the examples that were using any other initialization type
accordingly, using mock class and method names if necessary.
@jhlegarreta
DOC: Update itk::N4BiasFieldCorrectionImageFilter::Sharpen definition
55ea845 
Update `itk::N4BiasFieldCorrectionImageFilter::Sharpen` definition to
match the current one:
https://github.com/InsightSoftwareConsortium/ITK/blob/828453d1bf61c487310d2d8c9570093e08798a40/Modules/Filtering/BiasCorrection/include/itkN4BiasFieldCorrectionImageFilter.hxx#L261
@jhlegarreta jhlegarreta force-pushed the UpdateCodingStyleGuidelines branch from 5f6ae91 to 55ea845 Compare December 13, 2021 23:20
dzenanz
dzenanz approved these changes Dec 13, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Awesome!

N-Dekker
N-Dekker approved these changes Dec 14, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dzenanz dzenanz merged commit 781010a into InsightSoftwareConsortium:master Dec 14, 2021
@jhlegarreta jhlegarreta deleted the UpdateCodingStyleGuidelines branch December 14, 2021 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dzenanz dzenanz dzenanz approved these changes

@N-Dekker N-Dekker N-Dekker approved these changes

@thewtex thewtex Awaiting requested review from thewtex

@hjmjohnson hjmjohnson Awaiting requested review from hjmjohnson

@blowekamp blowekamp Awaiting requested review from blowekamp

Assignees

No one assigned

Labels

area:Appendices Issues affecting the Appendices part language:LaTeX Changes to LaTeX code type:BookStyle Changes to book style files type:Enhancement Improvement of existing methods or implementation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update the Code Style Guide section to match the .clang-format tool decisions

3 participants

@jhlegarreta @N-Dekker @dzenanz