bancorprotocol / contracts-solidity

Bancor Protocol Contracts
Other
840 stars 391 forks source link

Convert transferPositionAndCall to a more structured transferPositionAndNotify #618

Closed lbeder closed 3 years ago

lbeder commented 3 years ago

It was required for solidity-coverage, but in any case - it’s irrelevant.

On Wed, 12 May 2021 at 18:11 barakman @.***> wrote:

@.**** commented on this pull request.

In solidity/test/LiquidityProtection.js https://github.com/bancorprotocol/contracts-solidity/pull/618#discussion_r631237526 :

                                 {

from: recipient } ) );

  • expect(await testCall.num.call()).to.be.bignumber.equal(newNum);
  • expect(await testCall.str.call()).to.be.eql(newStr);
  • const protectionIds2 = await liquidityProtectionStore.protectedLiquidityIds(newOwner);
  • expect(protectionIds2.length).to.eql(1);
  • const transferEvent2 = await callback.transferEvent.call();

.cal() isn't required in any of the previous versions either (due to truffle)., hence this comment.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bancorprotocol/contracts-solidity/pull/618#discussion_r631237526, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVVYDXSIIVOWDTEJYO4EVDTNKZE5ANCNFSM44YSXXDQ .

barakman commented 3 years ago

It was required for solidity-coverage, but in any case - it’s irrelevant.

I never had to use it for solidity-coverage.

lbeder commented 3 years ago

I had to many many times 🤷‍♂️

On Wed, 12 May 2021 at 18:44 barakman @.***> wrote:

It was required for solidity-coverage, but in any case - it’s irrelevant.

I never had to use it for solidity-coverage.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bancorprotocol/contracts-solidity/pull/618#issuecomment-839972595, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVVYDTRVMS7AST2NZZTFZTTNK5APANCNFSM44YSXXDQ .