The cnetdiff application has been refactored to be callable; Makefile tests have been removed and replaced by gtests.

[kledmundson]

Description

The cnetdiff application has been refactored to be callable; Makefile tests have been removed and replaced by gtests.

Related Issue

https://github.com//issues/5322

How Has This Been Validated?

ISIS make file tests have been replaced with the following gtests.

CnetdiffReportFirst
CnetdiffReportFull

Results of ctest suite...

ctest -R Cnetdiff --output-on-failure
Test project
Start 1798:CompareNetsReportFirstDiff.FunctionalTestCnetdiffReportFirst
1/2 Test #1798: CompareNetsReportFirstDiff.FunctionalTestCnetdiffReportFirst ... Passed 0.74 sec
Start 1799: CompareNetsReportFull.FunctionalTestCnetdiffReportFull
2/2 Test #1799:
CompareNetsReportFull.FunctionalTestCnetdiffReportFull ......... Passed 0.56 sec

100% tests passed, 0 tests failed out of 2
Total Test time (real) = 1.50 sec

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Documentation change (update to the documentation; no code change)
• Breaking change (fix or feature that would cause existing functionality to change)
• Infrastructure change (changes to things like CI or the build system that do not impact users)

Checklist:

• I have read and agree to abide by the Code of Conduct
• I have read the CONTRIBUTING document.
• My change requires a change to the documentation and I have updated the documentation accordingly.
• I have added tests to cover my changes.
• I have added myself to the .zenodo.json document.
• I have added any user impacting changes to the CHANGELOG.md document.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

• I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

[acpaquette]

Overall looks great! Couple small things to address

[acpaquette]

Can this be moved to changed? If there is no Changed section feel free to add a new header titled Changed

[kledmundson]

Done.

[kledmundson]

Thanks for the very quick review Adam. I've made the changes and have hopefully addressed your suggestions. Ken

…

On Wed, Nov 1, 2023 at 12:57 AM acpaquette ***@***.***> wrote: ***@***.**** commented on this pull request. Overall looks great! Couple small things to address ------------------------------ In CHANGELOG.md <#5323 (comment)>: > @@ -45,7 +45,7 @@ release. outputsuffix, both, or neither for naming convention purposes. [#5162](#5162) ### Added -- Added rclone to run dependencies in meta.yaml [#5183](#5183) +- Cnetedit has been refactored to be callable; old Makefile tests have been removed and replaced by gtests. Issue: [#5322](#5322), Can this be moved to changed? If there is no Changed section feel free to add a new header titled Changed ------------------------------ In isis/src/control/apps/cnetdiff/cnetdiff.cpp <#5323 (comment)>: > + ignorekeys += PvlKeyword("DateTime", "true"); + } + + // compare ControlNetVersioners + Compare(cnv1, cnv2); + + PvlGroup differences("Results"); + if (filesMatch) { + differences += PvlKeyword("Compare", "Identical"); + } + else { + differences += PvlKeyword("Compare", "Different"); + differences += PvlKeyword("Reason", differenceReason); + } + + log.addGroup(differences); Could any addition to the log be changed to log.addLogGroup. This function logs to the app at the same time as the group is added to the keyword. With that you can remove the logging loop in the application main.cpp ------------------------------ In isis/src/control/objs/ControlNetDiff/ControlNetDiff.cpp <#5323 (comment)>: > + + ControlNetVersioner cnv1(net1Name); + ControlNetVersioner cnv2(net2Name); + + return compare(cnv1, cnv2, net1Name, net2Name); + } + + + /** + * Compare two Control Networks in ControNetVersioner form, and return their + * differences. + * + * @param cnv1 first Control Network + * @param cnv2 second Control Network + * @param net1Name FileName of the first Control Network + * @param net2Name FileName of the second Control Network + * @return Pvl The collection of all differences + */ + Pvl ControlNetDiff::compare(ControlNetVersioner &cnv1, ControlNetVersioner &cnv2, + FileName &net1Name, FileName &net2Name) { I don't really understand the necessity for this change. What does exposing the new function do that the old signature prevented? — Reply to this email directly, view it on GitHub <#5323 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABUGSUUDCAHRDTH2BF7JMHLYCH6E7AVCNFSM6AAAAAA6XXKFPKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBXG4YTQOJRHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>