Skip to content

QNTM-2446 Rename select UI nodes to provide distinction from ZT nodes#8368

Merged
alfarok merged 1 commit intoDynamoDS:masterfrom
alfarok:master
Dec 6, 2017
Merged

QNTM-2446 Rename select UI nodes to provide distinction from ZT nodes#8368
alfarok merged 1 commit intoDynamoDS:masterfrom
alfarok:master

Conversation

@alfarok
Copy link
Contributor

@alfarok alfarok commented Nov 30, 2017 •
edited
Loading

Purpose

QNTM-2446

This PR renames:

  • List.Create --> List Create
  • Function.Apply --> Apply Function
  • Function.Compose --> Compose Function

The purpose of doing this is so that they appear visually and syntactically distinct from other ZT nodes that can be called via DesignScript. Old graphs abiding by the old naming convention will be migrated but will still appear with the old name on the node due to the recent consolidation of name & nickname, however this is a known issue. The library will only display the newest naming convention.

Testing

I have added 1 new unit test verifying a successful migration of Function.Apply and Function.Compose from 0.9 to 2.0. There was already an existing test verifying this for List.Create that can be view here.

image

Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@mjkkirschner

@mjkkirschner
Copy link
Member

mjkkirschner commented Nov 30, 2017 •
edited
Loading

this looks good to me - was there more work to do here?
I would also look on the AVP side here - because of the rename I think there might be issues both in CoGs and AVP.

Please check all these issues and maybe write tests: (sorry I did not think of these issues at grooming)

  • when AVP associates a nodeView with a node it uses various names, check for instance if the list create node still attaches the correct view when uploaded.
  • CoGS has a list of aliased names for dynamo nodes, list.create may be one of these - the name in cogs might need to change or grow to include both names - @gregmarr will know more.

@alfarok
Copy link
Contributor Author

alfarok commented Dec 4, 2017

@mjkkirschner @gregmarr I looked into the repercussions of the renaming and all 3 node now return null on the AVP CoGS route but appear with the proper nodeView. It also seems Function.Apply and Function.Compose never functioned as expected and always returned null even prior to the renaming.

@mjkkirschner
Copy link
Member

mjkkirschner commented Dec 4, 2017

@alfarok CoGS /TS does not handle function passing so that makes sense for apply and compose.

@alfarok
Copy link
Contributor Author

alfarok commented Dec 6, 2017

@mjkkirschner I tried updating List.Create in AVP in the NodeFactory.ts mapping file and it didn't seem to influence anything. List.Create works without any changes and even with the changes is still showing up with the dot naming. Also not sure if this is expected but all the objects are nested an additional level from what is produced in Dynamo. Since the AC doesn't include the AVP work I am going to merge this and file another task to take a closer look if that is okay?

image

@alfarok alfarok removed the WIP label Dec 6, 2017
@alfarok alfarok merged commit c09383f into DynamoDS:master Dec 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alfarok @mjkkirschner