DYN-6394: Add legacy trace warning when opening workspace #14628
Conversation
aparajit-pratap
requested review from
pinzart90,
mjkkirschner and
saintentropy
avidit
changed the title
| @@ -44,6 +45,8 @@ public class EngineController : LogSourceBase, IAstNodeContainer, IDisposable | |||
| /// </summary> | |||
| internal static event Action VMLibrariesReset; | |||
|
|
|||
| internal Version WorkspaceVersion { get; set; } | |||
There was a problem hiding this comment.
the name of this property confused me at first glance, shouldn't this be called something like engine version? I would think that WorkspaceVersion is the version of the currently loaded workspace.
There was a problem hiding this comment.
I moved this property from WorkspaceModel to EngineController as part of this latest refactoring as I need it before the workspace is initialized. It's meant to be the Dynamo version in which the workspace was initially created.
There was a problem hiding this comment.
Okay, I guess my confusion stems from the fact that I see it get set from the AssemblyHelper? Maybe on EngineController it could have a different name?
There was a problem hiding this comment.
It still gets serialized with the workspace - correct?
There was a problem hiding this comment.
It gets set from AssemblyHelper only if there's no version serialized with the workspace being read. Yes, it still gets serialized as part of DynamoPreferencesData. I can think of another name.
| @@ -313,14 +317,12 @@ where childNode.Name.Equals(sessionXmlTagName) | |||
| var callsiteId = string.Empty; | |||
| if (child.HasAttribute(Configurations.CallSiteID)) | |||
| { | |||
| callsiteId = child.GetAttribute(Configurations.CallSiteID); | |||
| containsTraceData = true; | |||
| return loadedData; | |||
There was a problem hiding this comment.
so now, we just return an empty list no matter what, but we make a note that there was at least 1 node with trace data?
There was a problem hiding this comment.
I renamed the variable from containsTraceData to containsLegacyTraceData to make it clearer. This flag is only used to trigger the toast notification when opening an old workspace.
src/DynamoCore/Models/DynamoModel.cs
Outdated
| @@ -2323,6 +2323,10 @@ private void SetPeriodicEvaluation(WorkspaceModel ws) | |||
| var currentHomeSpace = Workspaces.OfType<HomeWorkspaceModel>().FirstOrDefault(); | |||
| currentHomeSpace.UndefineCBNFunctionDefinitions(); | |||
|
|
|||
| // This is to handle the case of opening a JSON file that does not have a version string | |||
| EngineController.WorkspaceVersion = dynamoPreferences.Version == | |||
There was a problem hiding this comment.
did you see any of these files?
There was a problem hiding this comment.
No, I haven't seen any. This comment has been copied from deleted lines of code below.
| // Ignore revision number. | ||
| var ver = workspace.WorkspaceVersion; | ||
| return new Version(ver.Major, ver.Minor, ver.Build, 0); | ||
| } |
Purpose
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Add legacy trace warning when opening workspace.
FYI @Amoursol for warning message.