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

DYN-7082: Improvements to node List.IndexOf #15440

Merged
merged 6 commits into from
Aug 26, 2024

Conversation

reddyashish
Copy link
Contributor

@reddyashish reddyashish commented Aug 22, 2024

Purpose

https://jira.autodesk.com/browse/DYN-7082
In reference to #15268, to avoid negative index values being returned, we modified IndexOf that will return null when the element is not found in the list.

Before:
Screenshot 2024-08-27 at 7 52 58 AM

After:
Screenshot 2024-08-27 at 7 29 16 AM

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.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

DYN-7082: Improvements to node List.IndexOf

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

@github-actions github-actions bot changed the title Adding a new node List.StringifyIndexOf DYN-7082: Adding a new node List.StringifyIndexOf Aug 22, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7082

Copy link

github-actions bot commented Aug 22, 2024

UI Smoke Tests

Test: success. 11 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests

@mjkkirschner
Copy link
Member

why does this return a string?

@reddyashish
Copy link
Contributor Author

Because we want to return null(and not -1) if the element is not found in the list. After this change, #15205, passing -1 as index would get the last element from the list.

@mjkkirschner
Copy link
Member

mjkkirschner commented Aug 22, 2024

yeah I'm not getting it -
Because we want to return null(and not -1) - so just return null from a regular index function that returns int|null - what does this function returning a string|null index help?

@reddyashish
Copy link
Contributor Author

reddyashish commented Aug 26, 2024

Got it, my bad for misunderstanding. Changes should be fine now.

@QilongTang QilongTang merged commit 682c830 into DynamoDS:master Aug 26, 2024
23 of 24 checks passed
@QilongTang QilongTang changed the title DYN-7082: Adding a new node List.StringifyIndexOf DYN-7082: Modifying node List.StringifyIndexOf Aug 26, 2024
@QilongTang QilongTang added this to the 3.3 milestone Aug 26, 2024
@QilongTang
Copy link
Contributor

@reddyashish Maybe add screenshot for behavior diff?

@reddyashish reddyashish changed the title DYN-7082: Modifying node List.StringifyIndexOf DYN-7082: Improvements to node List.IndexOf Aug 27, 2024
@QilongTang
Copy link
Contributor

Updated PR description

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