search

EOSIO / eosio.contracts

Smart contracts that provide some of the basic functions of the EOSIO blockchain
https://eosio.github.io/eosio.contracts/latest
MIT License
325 stars 574 forks source link

Fix v2.0.5 unit tests compile error by adding create_yield_function #488

Closed MrToph closed 4 years ago

MrToph commented 4 years ago

Change Description

Compiling the unit tests when running EOSIO v2.0.5 results in the error described in issue #487 I've seen them being wrapped in abi_serializer::create_yield_function calls in the eosio/eos repo, so I did the same here.

There's no compile error now but I run into a boost linking error on Mac, see the issue. Not sure if this is related. Would be great if someone could test

revl commented 4 years ago

@MrToph, thanks for submitting the fixes. I did not encounter any linking errors. The jury is still out though on which one is the right thing to do: fixing the unit tests or restoring backward compatibility. If they decide to not fix it on the EOSIO side, I'll merge the PR.

revl commented 4 years ago

The decision was to restore backward compatibility in abi_serializer. However, these fixes still need to be merged, only to a release branch. Would you be able to create a clone of this PR against the release/1.9.x branch in this repo? (I tried to change the base branch for this PR but that didn't work out)

And while you're at it, could you please apply a similar fix to line 141 in tests/eosio.msig_tests.cpp? That piece of code is commented out, but it would be nice to keep it compilable in case it's ever uncommented.

MrToph commented 4 years ago

HI @revl , is it even worth continuing this PR if the decision to make it backwards-compatible was made?

revl commented 4 years ago

Yes, even though the compatibility was restored, the methods with old signatures were reintroduced as [[deprecated]]. This PR contains changes that reflect the new way of setting the time limit - using a yield function. So the changes from this PR need to be merged, but they must go into the release/1.9.x branch, not master. Would you be able to create a new PR against release/1.9.x with the same fixes?

MrToph commented 4 years ago

I see, okay I'll create a PR branching off release/1.9.x

revl commented 4 years ago

Thank you!

revl commented 4 years ago

We don't merge directly to master, so I created https://github.com/EOSIO/eosio.contracts/pull/501 to merge an identical change that had been committed earlier into release/1.9.x.