Skip to content

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