Update regression tests that use obsolete attribute values.

[biagas]

If you look at the testsuite log file, there are many warnings such as:
'backgroundFlag' is obsolete. Use 'opaqueColor'.
'foregroundFlag' is obsolete. Use 'meshColor'.
'specifyTextColor1' is obsolete. Trying to accommodate.

While I understand in part leaving them in the test, to ensure old scripts don't completely break when attributes change, the regression tests also serve as examples of how to do things in VisIt. As such wouldn't it be better to update any test using obsolete values?

Cleaning these up would also make it easier to find actual problems with tests, as it would reduce the warning messages in the log file.

If we still wanted to ensure old scripts work (via testing), we could possibly introduce a new test, with the sole purpose of testing obsolete things. 'OldAtts' test or something like that. Say that the OldAtts test existed when the way that Mesh plot's colors were specified. Any existing mesh plot test would have been updated to the new way, and a Mesh plot color specific test would be added to the OldAtts test, perhaps with a 'remove by version' tag. It could be a bit of a maintenance headache though.

[markcmiller86]

As such wouldn't it be better to update any test using obsolete values?

Yes.

If we still wanted to ensure old scripts work (via testing), we could possibly introduce a new test, with the sole purpose of testing obsolete things.

Great idea! 👍🏻

[markcmiller86]

BTW...if we have a remove by version tag, that would be cool too. There are a couple of places in VisIt C++ code I've done that. I think we need to be committed to removing old code and so we'd want to not only remove testing of the old logic but to also remove that old logic from our code. I discuss an idea for this here

BTW...just happened to be looking at this yesterday and by lines of code count, I think the only developer among us who consistently removes (old) code is @biagas and I think she should be highly regarded for that and that we should all try to follow her example. Removing unneeded code is at least as important if not more so than adding new code for new features. It is what helps keep the product maintainable/sustainable.

[brugger1]

We should probably go back and remove commands that have been obsoleted a long time ago (like 10+ years ago).

[biagas]

To locate files that have code that can possibly be removed immediately: grep for VersionLessThan.
It is used in the ProcessOldVersions method for attributes.
For Py atts, can grep for Try and handle legacy fields but that may not be used by all.
Just need to decide the immediate cut-off version. anything for version < 2.0 ? 2.13? 3.0?

[biagas]

For future I'm thinking we can wrap all ProcessOldVersions and python legacy field handling in

#if VISIT_VERSION_GE(x,y,z)
#error Time to remove obsolete field support
#else 
  // handle newly obsoleted field
#endif 

Just need a standard policy: set removal for next major release? two major releases from current?

[markcmiller86]

Makes sense to me. Do you see any value in trying to both have compile time behavior you describe but also be able to somehow disable this via either a single, global CMake flag or (more complex) code-block-specific CMake flags?

[biagas]

I'm not quite certain how something like this could be handled by CMake. It seems very VisIt version specific, and dependent upon which VisIt version a field is removed/renamed. Could you explain a bit more your ideas for CMake usage?

[markcmiller86]

Whoops...didn't mean to suggest there is any support for something like this in CMake itself. Just that we'd define CMake-time, or make make-time variables that allow for disablement of the disablements 😮 if that makes sense.

[biagas]

You mean a code-parser that would look for something like "remove this code block at version x.y.z", and automagically remove that code block? That would be something great to run outside of the build cycle, and could be done once at each change-in-major version. It would also remove the onus from whichever developer happened to be the first to compile once the magic version number is reached. Someone would still have to vet the changes and create a PR, though.

[markcmiller86]

You mean a code-parser that would look for something like "remove this code block at version x.y.z",

No, I am not suggesting anything that sophisticated. All I am asking is whether it makes sense, in addition to what you describe in the above comment that we have the ability to override the effect of the #error Time to remove obsolete field support at either CMake-time or maybe make-time by providing appropriate CMake or make variables like so...

cmake -DVISIT_OVERRIDE_OBSELETE_CODE:BOOL=Yes ...

or

make OVERRIDE_OBSELETE_CODE=Yes

So, instead of using VISIT_VERSION_GE directly we'd have a new macro

#ifndef VISIT_OVERRIDE_OBSOLETE_CODE
#define VISIT_OBSOLETE_AT_VERSION(A,B,C) VISIT_VERSION_GE(A,B,C)
#else
#define VISIT_OBSOLETE_AT_VERSION(A,B,C) 0
#endif 
.
.
.
#if VISIT_OBSOLETE_AT_VERSION(4,0,0)
#error This code is obsolete in this version. Please remove it.
#else
// old code here
#endif 

such that, for example, something like make VISIT_OVERRIDE_OBSOLETE_CODE=1 has the effect of disabling the #if portion, above