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

fix: show cli events correctly #645

Merged
merged 1 commit into from
Nov 6, 2018
Merged

Conversation

nicosantangelo
Copy link
Contributor

No description provided.

@abarmat abarmat merged commit f78cd4a into master Nov 6, 2018
@abarmat abarmat deleted the fix/mkcli-blockchain-events branch November 6, 2018 19:00
Copy link
Contributor

@nachomazzara nachomazzara left a comment

Choose a reason for hiding this comment

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

Thanks Nico! just a few comments

log += `with id ${args.id}`
break
case events.publicationSuccessful:
case eventNames.AuctionSuccessful:
case eventNames.OrderSuccessful:
log += `with id ${args.id} and winner ${args.winner}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember for OrderSuccessful has the winner as buyer

log += `with id ${args.id} and buyer ${event.args.winner || event.args.buyer}` 

Ref: https://github.com/decentraland/marketplace-contracts/blob/master/contracts/marketplace/MarketplaceStorage.sol#L75

log += `with data ${args.data}`
break
case events.parcelTransfer:
case eventNames.Transfer:
log += `to ${args.to}`
Copy link
Contributor

Choose a reason for hiding this comment

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

IDK if it worth but FYI, for Estates the _to arg it _to

nachomazzara added a commit that referenced this pull request Nov 6, 2018
@nachomazzara nachomazzara restored the fix/mkcli-blockchain-events branch November 6, 2018 21:28
@nicosantangelo nicosantangelo deleted the fix/mkcli-blockchain-events branch November 15, 2018 11:39
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.

3 participants