Skip to content

Conversation

@Morantron
Copy link
Collaborator

Working on the multitransfers feature, I needed to use TransferSourcesOptions. In the context of that feature, when using TransferSourcesOptions, we don't know the destination accountable beforehand.

This PR removes the dependency on a destination accountable.

@Morantron Morantron force-pushed the fix/remove-destination-accountable-from-transfer-sources-options branch from 3555d49 to f603758 Compare April 10, 2019 14:16
@Morantron Morantron requested a review from sauloperez April 10, 2019 14:17
@enricostano
Copy link
Contributor

Related 14eaad8

@sauloperez @sseerrggii any thought?

@sseerrggii
Copy link
Contributor

Works fine tested on staging to:

Transfer time link from:

  • Member
  • Post
  • Link from Post showed on member profile

Some checks failed on travis

@Morantron Morantron force-pushed the fix/remove-destination-accountable-from-transfer-sources-options branch from f603758 to 8e5d6ac Compare April 11, 2019 15:08
@markets
Copy link
Collaborator

markets commented Apr 11, 2019

@Morantron if you rebase current develop, you will get code-coverage reports 💫

@Morantron Morantron force-pushed the fix/remove-destination-accountable-from-transfer-sources-options branch from 8e5d6ac to 5f56bec Compare April 12, 2019 07:26
@Morantron Morantron force-pushed the fix/remove-destination-accountable-from-transfer-sources-options branch from 5f56bec to af7e3a6 Compare April 12, 2019 07:34
@Morantron
Copy link
Collaborator Author

Morantron commented Apr 12, 2019

lol, i deleted code/branching, and this somehow reduced reported overall coverage 🤔

also deleted some tests, guess it makes sense

@sseerrggii
Copy link
Contributor

@enricostano can we Merge it?

@enricostano enricostano merged commit 91209a5 into develop Apr 16, 2019
@enricostano enricostano deleted the fix/remove-destination-accountable-from-transfer-sources-options branch April 16, 2019 08:08
@sseerrggii sseerrggii mentioned this pull request May 16, 2019
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.

5 participants