Skip to content

remove/useless-label - #1298

Closed
decanus wants to merge 3 commits into
threshold-network:masterfrom
decanus:remove/useless-label
Closed

remove/useless-label#1298
decanus wants to merge 3 commits into
threshold-network:masterfrom
decanus:remove/useless-label

Conversation

@decanus

@decanus decanus commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

removes useless labels, change the return label to be compliant with argotorg/solidity#7534

@decanus
decanus requested a review from mhluongo January 24, 2020 03:41
@mhluongo
mhluongo requested a review from ngrinkevich January 24, 2020 03:50

@mhluongo mhluongo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍, leaving it to @ngrinkevich for another review and merge.

Keep em coming @decanus 😉

@decanus

decanus commented Jan 24, 2020

Copy link
Copy Markdown
Contributor Author

@mhluongo only got around to fixing it here, should get around to doing it everywhere else.

@Shadowfiend

Copy link
Copy Markdown
Contributor

The labels aren't useless, they're how the struct gets assembled on the Go side, which is why the Go build failed in this case. As well, pulling names out of code and adding them to the docs isn't something we're likely to be doing. If a name is in the docs, it should be in the code unless there's no way to specify it there. Code is best considered the ultimate source of truth in these matters.

With that said, thanks for adding the multiple @return docs here and pointing out the relevant Solidity change; I hadn't seen that one come through -- opening a separate PR that updates the multi-returns I noticed and puts proper docs on 'em while we're in there.

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