Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support namespace qualifiers in classes used in Imperative blocks #9252

Merged
merged 10 commits into from
Nov 19, 2018

Conversation

aparajit-pratap
Copy link
Contributor

@aparajit-pratap aparajit-pratap commented Nov 15, 2018

Purpose

Support namespace qualifiers in classes used in Imperative blocks: https://jira.autodesk.com/browse/QNTM-5647
image
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 @QilongTang @ColinDayOrg

FYIs

@smangarole

@aparajit-pratap aparajit-pratap changed the title Support namespace qualifiers in classes used in Imperative blocks [WIP] Support namespace qualifiers in classes used in Imperative blocks Nov 16, 2018
@@ -670,6 +724,38 @@ public static AssociativeNode CreateNodeByCombiningIdentifiers(IList<Associative
return newIdentList;
}

public static AST.ImperativeAST.ImperativeNode CreateNodeByCombiningIdentifiers(IList<AST.ImperativeAST.ImperativeNode> nodeList)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

summary tag for public method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use an IEnumerable for a public method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, did not see it was an IList

@ColinDayOrg
Copy link
Contributor

Generally, LGTM, although it is a little difficult to parse some of the changes. Also, there are some sections of code that were commented out, but left in, was this for any specific reason?

@aparajit-pratap aparajit-pratap changed the title [WIP] Support namespace qualifiers in classes used in Imperative blocks Support namespace qualifiers in classes used in Imperative blocks Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants