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

Add New Node GetNumber #12332

Merged
merged 11 commits into from
Dec 15, 2021
Merged

Add New Node GetNumber #12332

merged 11 commits into from
Dec 15, 2021

Conversation

chuongmep
Copy link
Contributor

@chuongmep chuongmep commented Nov 26, 2021

Purpose

Support Get Number In String.This is very helpful, I have come across forum posts within the past year, users need it for object separation, counting and statistics, checking,...
image
Node In Action
image

Declarations

Check these if you believe they are true

  • The codebase 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.

Reviewers

@QilongTang @mjkkirschner

FYIs

@Amoursol

@QilongTang
Copy link
Contributor

Our team can add node icon later but so far I feel this would be a good addition to the existing node base. Thank you for proposing this.

@QilongTang QilongTang added this to the 2.14.0 milestone Dec 1, 2021
@Amoursol
Copy link
Contributor

Amoursol commented Dec 2, 2021

This is awesome @chuongmep - thank you for submitting this and making Dynamo better! 🙌

@QilongTang
Copy link
Contributor

Regression found DSCoreNodesTests.StringTests.GetNumber

Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

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

LGTM

@QilongTang
Copy link
Contributor

Seems Dynamo SelfServe PR check not triggered correctly for this PR. I am running on demand job at https://master-15.jenkins.autodesk.com/view/DYN/job/DYN-DevCI_Self_Service/917/ and monitoring the results. FYI: @DynamoDS/dynamo

@QilongTang
Copy link
Contributor

I see the following regression result, can you take another look?

Failed : DSCoreNodesTests.StringTests.GetNumber
  Expected string length 12 but was 5. Strings differ at index 2.
  Expected: "01 Level 200"
  But was:  "01200"

@chuongmep
Copy link
Contributor Author

@QilongTang sorry about that, updated.

@QilongTang
Copy link
Contributor

@chuongmep Thanks, I have run the self CI again and everything pass now. Once the PR checks are passing, I will merge this

@QilongTang QilongTang merged commit 4a3b48a into DynamoDS:master Dec 15, 2021
@chuongmep chuongmep mentioned this pull request Apr 6, 2022
8 tasks
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.

4 participants