Skip to content

STYLE: Make work unit naming consistent#2695

Merged
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
ebrahimebrahim:consistent_variable_naming_for_work_units
Aug 17, 2021
Merged

STYLE: Make work unit naming consistent#2695
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
ebrahimebrahim:consistent_variable_naming_for_work_units

Conversation

@ebrahimebrahim
Copy link
Copy Markdown
Contributor

@ebrahimebrahim ebrahimebrahim commented Aug 16, 2021

A new multi-threading mechanism was introduced in commit 2075084,
and this led to some variables being stuck in the old naming system.
We see thread counts and IDs showing up in variable names where
the new system should reference work unit counts and IDs.
This commit renames those variables.

Resolves #86


PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

@github-actions github-actions Bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Aug 16, 2021
@thewtex thewtex requested a review from dzenanz August 16, 2021 17:58
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.

Looks mostly good.

Please squash all changes into one commit and rebase on master.

Comment thread Modules/Core/Common/include/itkDomainThreader.hxx Outdated
Comment thread Modules/Core/Common/test/itkSTLThreadTest.cxx Outdated
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.

To be squashed upon merge, if not squashed before.

A new multi-threading mechanism was introduced in commit 2075084,
and this led to some variables being stuck in the old naming system.
We see *thread* counts and IDs showing up in variable names where
the new system should reference *work unit* counts and IDs.
This commit renames those variables.

Resolves InsightSoftwareConsortium#86
@ebrahimebrahim ebrahimebrahim force-pushed the consistent_variable_naming_for_work_units branch from 4630635 to 03024da Compare August 16, 2021 20:18
@dzenanz dzenanz merged commit 03024da into InsightSoftwareConsortium:master Aug 17, 2021
@ebrahimebrahim ebrahimebrahim deleted the consistent_variable_naming_for_work_units branch August 17, 2021 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Name job pieces consistently when calling numberOfWorkUnits()

2 participants