[CHERRY-PICK] DYN-9386 - Remove nodes marked with [NodeObsolete] in Dynamo 1.3.4#16709
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9386
|
SelfServe Passed, merging. |
| /// <summary> | ||
| /// A class for storing structure point analysis data. | ||
| /// </summary> | ||
| [IsVisibleInDynamoLibrary(false)] |
There was a problem hiding this comment.
In the future, is it feasible to also mark with C# Obsolete attribute to get compiler warning for those who use/call from code and not only visually, from graphs?
Otherwise it's rather cumbersome to check custom attributes on everything in an entire code base.
There was a problem hiding this comment.
Or perhaps to derive the NodeObsolete attribute from the C# Obsolete attribute to get compiler warnings?
There was a problem hiding this comment.
Or perhaps there is an already made analyzer that we should be using?
There was a problem hiding this comment.
NodeObsolete and Obsolete have 2 different use cases and we can try to apply Obsolete in addition to NodeObsolete, where it makes sense.
NodeObsolete is used to show warnings in the info bubble in Dynamo. So it is a runtime notification to graph authors.
Obsolete is the C# annotation that generates the compiler warnings. So it is a compile time warning for Dynamo developers.
I'm not sure how much overlap there is between these two scenarios, but when there is, we should be using both attributes.
There was a problem hiding this comment.
Our use case was this: we were using internally in DynamoRevit some functions marked NodeObsolete for some years now, but we never knew because there were no compiler warnings, and other than compiler there is nothing for code usage, then close to code split they got removed from Dynamo Core.
I added you as a reviewer as well for a proposal long term solution DynamoRevit #3265
There was a problem hiding this comment.
*proposal for long term solution for detecting these early - sorry for confusion, the actual solution for this particular situation is still WIP
There was a problem hiding this comment.
I will file a task to either mark all occurrences of NodeObsolete with NodeObsolete as well, or to have NodeObsolete inherit from Obsolete, which might be more failsafe, but I will need to discuss with the team as to the best approach.
Apologies for the misunderstanding.
Purpose
DYN-9386 - Remove nodes marked with [NodeObsolete] in Dynamo 1.3.4
Cherry-pick of two upstream PRs into RC4.0.0_master:
Release Notes
Removes 18 user-facing node functions that were marked with [NodeObsolete] in Dynamo 1.3.4:
CoreNodes (6 items):
ReadImage, LoadImageFromPath, ReadText, WriteImage, ExportToCSV, String.FromObject
Analysis (11 items):
PointData: ValueLocations, Values, ByPointsAndValues
SurfaceData: Surface, ValueLocations, Values, BySurfaceAndPoints, BySurfacePointsAndValues
VectorData: ValueLocations, Values, ByPointsAndValues
DSOffice (1 item):
Excel.Read(string, string)