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

Feat/groundwork for targets to new dao #3659

Merged
merged 7 commits into from Jul 31, 2018

Conversation

Projects
None yet
2 participants
@kikito
Copy link
Member

commented Jul 31, 2018

These are a few uncontroversial changes that are needed in order to merge feat/targets-to-new-dao

@kikito kikito changed the base branch from master to next Jul 31, 2018

@kikito kikito force-pushed the feat/groundwork-for-targets-to-new-dao branch from 1314b58 to 0793998 Jul 31, 2018

if err and migration.ignore_error then
if type(migration.ignore_error) == "boolean" or
(type(migration.ignore_error) == "string" and
string.find(tostring(err), migration.ignore_error)) then

This comment has been minimized.

Copy link
@hishamhm

hishamhm Jul 31, 2018

Member

I suggest changing this to string.find(tostring(err), migration.ignore_error, 1, true) so that it does a plain-text match, just so this doesn't fail mysteriously someday in the future when we have a message that happens to have a hyphen.

@kikito kikito force-pushed the feat/groundwork-for-targets-to-new-dao branch from 0793998 to 9f75df0 Jul 31, 2018

@hishamhm
Copy link
Member

left a comment

Review addressed, LGTM!

@hishamhm hishamhm merged commit 05adc40 into next Jul 31, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@hishamhm hishamhm deleted the feat/groundwork-for-targets-to-new-dao branch Jul 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.