COMP: Remove unused topCluster var from FiberTractMeasurements

[jhlegarreta]

Fix the unused topCluster variable warning in FiberTractMeasurements: remove the variable.

The topCluster variable was originally introduced in 798a1e5 and its use became no longer necessary in 9bdd1d3, where it was left behind.

Fixes:

Modules/CLI/FiberTractMeasurements/FiberTractMeasurements.cxx:563:10:
 warning: variable ‘topCluster’ set but not used [-Wunused-but-set-variable]
     bool topCluster = true;
          ^~~~~~~~~~

reported on the 3D Slicer CDash server in the context of preview builds on CentOS 7 with GCC 7:
https://slicer.cdash.org/viewBuildError.php?type=1&buildid=3210886

[jcfr]

The ivar topCluster was originally introduced in 798a1e5 (ENH: FBTractMeasurement: by default, print only specified hierarchy.) and became obsolete in 9bdd1d3 (FBTractMsr: correctly fix #36: by default print top and cluster groups, but not fiberbundles).

Consider removing the lines instead of commenting.

[jcfr]

Suggested commit message:

COMP: Remove unused topCluster ivar from FiberTractMeasurements

The topCluster variable was originally introduced in
798a1e5 (ENH: FBTractMeasurement: by default, print only specified hierarchy.)
and became obsolete in 9bdd1d3 (FBTractMsr: correctly fix #36: by default print
top and cluster groups, but not fiberbundles).

Fixes:
```
Modules/CLI/FiberTractMeasurements/FiberTractMeasurements.cxx:563:10:
 warning: variable ‘topCluster’ set but not used [-Wunused-but-set-variable]
     bool topCluster = true;
          ^~~~~~~~~~
```

[jcfr]

I suggest to avoid including the following in the commit message:

raised for example in:
https://slicer.cdash.org/viewBuildError.php?type=1&buildid=3210886

Since the buildid corresponding to 3210886 will be invalid a few months, I suggest to reference it only in the pull request or issue.

[jhlegarreta]

I suggest to avoid including the following in the commit message:
Since the buildid corresponding to 3210886 will be invalid a few months, I suggest to reference it only in the pull request or issue.

I am aware of that, but for the moment when/few weeks around a commit tries to address an issue, to me it is essential that we can openly see which the issue is or where it has been observed. And I dislike using GitHub comments to mention things that are integral to the history of the code, as we cannot see them in the git history.

[jcfr]

it is essential that we can openly see which the issue is or where it has been observed

Indeed, my understanding is that the warning copied in the commit message fulfills this role providing the context and the exact same information that is reported on CDash.

In practice, adding the a CDash link that becomes obsolete will mislead developers ...(Why is the link not working ? Is the server broken ? ...)

Adding something like this would be much more explicit:

Fixes the following warning reported on the Slicer CDash server in the
context of "preview" build done on Centos 7 with GCC 7:
...

[jhlegarreta]

I prefer to keep the link and be open about the context and reproducibility of the error. It is helpful at the time of reviewing a PR.

[jcfr]

In the commit message, I then suggest to include something like this:

Fixes the following warning reported on the Slicer CDash server in the
context of "preview" build done on Centos 7 with GCC 7:

```
Modules/CLI/FiberTractMeasurements/FiberTractMeasurements.cxx:563:10:
 warning: variable ‘topCluster’ set but not used [-Wunused-but-set-variable]
     bool topCluster = true;
          ^~~~~~~~~~
```

it is helpful at the time of reviewing a PR.

Exactly, while submitting the PR you then add a line like:

The warning fixed by this pull-request was reported at https://slicer.cdash.org/viewBuildError.php?type=1&buildid=3210886

[jhlegarreta]

Warnings are gone; test failures are expected.