Skip to content

ENH: Add script to update ITK testing macros names#1009

Merged
jhlegarreta merged 1 commit intoInsightSoftwareConsortium:masterfrom
jhlegarreta:AddUpdateTestingMacrosNamesScript
Jun 12, 2019
Merged

ENH: Add script to update ITK testing macros names#1009
jhlegarreta merged 1 commit intoInsightSoftwareConsortium:masterfrom
jhlegarreta:AddUpdateTestingMacrosNamesScript

Conversation

@jhlegarreta
Copy link
Copy Markdown
Member

@jhlegarreta jhlegarreta commented Jun 11, 2019

Add script to update ITK testing macros names.

Motivated by PR #999.

This script should ease the task to update the testing macros names in
remote modules when used together with the ApplyScriptToRemotes.sh script.

PR Checklist

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

Add script to update ITK testing macros names.

Motivated by PR InsightSoftwareConsortium#999.

This script should ease the task to update the testing macros names in
remote modules when used together with the
[ApplyScriptToRemotes.sh](https://github.com/InsightSoftwareConsortium/ITK/blob/master/Utilities/Maintenance/ApplyScriptToRemotes.sh)
script.
@jhlegarreta jhlegarreta added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation area:Remotes Issues affecting the Remote module labels Jun 11, 2019
@jhlegarreta
Copy link
Copy Markdown
Member Author

Tested as a stand-alone script. Working to apply it to remotes.

Related to this, I was wondering whether we should make the ApplyScriptToRemotes.sh script accept a varying number of additional input parameters in the event that the script that is applied to the remotes (i.e. its <script_filename> argument) requires input parameters. I guess this is doable, although it may not be easy.

I thought about it because to apply the above patch, I had to look at #999, see the changes in itkTestingMacros.h and manually code the changes to be applied by the script. I thought that an improvement over this would be to:

  1. Identify the commit and the file(s) that we are interested in to draw the changes from (e.g. using git diff and generating a file with those changes).
  2. Get the "changes to be applied" (e.g. change TEST_GET_SET_VALUE to ITK_TEST_SET_GET_VALUE) as an array of arguments that would be passed on to the script.

However, identifying the "changes to be applied" from the diff file to be applied to other files does not seem straightforward, to say the least.

So if we do not foresee the latter as doable, I do not see the necessity of making the ApplyScriptToRemotes.sh script accept a varying number of additional input parameters right now.

I am not willing to tackle this now (both modifying ApplyScriptToRemotes.sh to accept a varying number of additional input parameters, and a script to identify "changes to be applied" from a diff), just wondering whether you think it is doable/useful, in which case I'd open an issue.

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.

I did not try this.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Jun 11, 2019

Adding variable number of arguments to ApplyScriptToRemotes.sh is probably not hard. Extracting meaningful changes from a diff to be distilled into a change-script is almost certainly impossible.

Comment thread Utilities/Maintenance/UpdateTestingMacrosNames.sh
@jhlegarreta jhlegarreta merged commit 794d37a into InsightSoftwareConsortium:master Jun 12, 2019
@jhlegarreta jhlegarreta deleted the AddUpdateTestingMacrosNamesScript branch June 12, 2019 00:21
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…dUpdateTestingMacrosNamesScript

ENH: Add script to update ITK testing macros names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Remotes Issues affecting the Remote module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants