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

Allow users to specify the old database themselves using current default logic #267

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from

Conversation

ArlonAntonius
Copy link
Member

Fixes #212

This is a possible solution, but I'm not completely sure yet if this is the best solution.
It's sort of a breaking change; if people have custom logic and have not specified oldDatabase, but have specified oldUsername, than this is becoming an issue.

@codecov
Copy link

codecov bot commented Oct 30, 2022

Codecov Report

Merging #267 (d11733f) into 2.x (4d06a38) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##                2.x     #267   +/-   ##
=========================================
  Coverage     98.97%   98.97%           
- Complexity      290      291    +1     
=========================================
  Files            98       98           
  Lines           777      779    +2     
=========================================
+ Hits            769      771    +2     
  Misses            8        8           

Copy link
Contributor

@luceos luceos left a comment

Choose a reason for hiding this comment

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

I feel the same struggle with the proposed code as you do. But no matter how I look at it, there doesn't seem a sweeter alternative without going full Monty with some complex implementation where state is carried over outside into event listeners and back into the database mutation logic.

I am okay with this implementation, but yes this is a breaking change and thus needs to be documented and requires at least a minor tag..

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.

Possibly dropping wrong database
2 participants