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

use existing library function to lookup previous name hints #11379

Merged
merged 6 commits into from
Jan 7, 2021

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Jan 5, 2021

Purpose

I found that this PR:
#9306

did indeed get migrations working using the migration.xml and AKA attributes - but not consistently with xml migrations.

This PR uses an existing function in library services to first check for an exact match using the full function signature (including parameter types) - and then falls back to checking for shortNames without parameters. This is more consistent with xml migration.

I've manually tested changing the name of a ZT node in the DSOffice namespace, adding that new function(without parameters) to the DSOffice.migrations.xml and all works as expected.

TODO:

  • add a test for this migration
  • investigate how old tests pass, likely they are not precise enough. The tests did not include a case of migrating a type with parameters using a migration name hint that did not include those parameters - modified the test migration xml to include this case.

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

@mjkkirschner mjkkirschner changed the title use existing library function to lookup previous name hints WIP use existing library function to lookup previous name hints Jan 5, 2021
@mjkkirschner mjkkirschner changed the title WIP use existing library function to lookup previous name hints use existing library function to lookup previous name hints Jan 5, 2021
@mjkkirschner mjkkirschner added PTAL Please Take A Look 👀 and removed WIP labels Jan 5, 2021
add migration xml to ffitarget
change method name in lib services
@mjkkirschner
Copy link
Member Author

@aparajit-pratap I've added a new test case using FFITarget with a new class - it checks:

  1. that a node with extra parameters is migrated correctly to a new method, with a new name - both methods still exist.
  2. a method name is changed and the old method is missing.
  3. A node is migrated from one overload to another with the same name but a different number of parameters.

@mjkkirschner mjkkirschner merged commit f78cf1b into DynamoDS:master Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PTAL Please Take A Look 👀
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants