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

Refactor: Fix name conflict of struct Dispute and event Dispute #92

Merged
merged 3 commits into from
Aug 7, 2018
Merged

Refactor: Fix name conflict of struct Dispute and event Dispute #92

merged 3 commits into from
Aug 7, 2018

Conversation

0xferit
Copy link
Member

@0xferit 0xferit commented Aug 7, 2018

This name conflict becomes a problem when both CentralizedArbitrator (has struct Dispute) and Arbitrable (has event Dispute) contracts inherited, which is required in AppealableArbitrator. Discovered this while working on #3.

So following contract doesn't compile:

contract AppealableArbitrator is CentralizedArbitrator, Arbitrable

To fix it I renamed event Dispute to event DisputeCreated in Arbitrable but maybe you can find a better name.

This name conflict becomes a problem when both CentralizedArbitrator and Arbitrable contracts inherited, which is required in AppealableArbitrator.
@0xferit 0xferit requested review from clesaege and dannybabbev August 7, 2018 10:55
@0xferit 0xferit changed the title Refactor: Fix name conflict with struct Dispute Refactor: Fix name conflict of struct Dispute and event Dispute Aug 7, 2018
@clesaege
Copy link
Member

clesaege commented Aug 7, 2018

Events can't be renamed because some code depends of it.
But types can be.

Copy link
Member

@clesaege clesaege left a comment

Choose a reason for hiding this comment

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

Events can't be renamed as code depends of it.

@0xferit 0xferit dismissed clesaege’s stale review August 7, 2018 14:19

I reverted the fix and proposed another one.

@0xferit 0xferit requested a review from clesaege August 7, 2018 14:20
@clesaege clesaege merged commit 175a67c into kleros:master Aug 7, 2018
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.

2 participants