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

Assert output types #514

Merged
merged 3 commits into from
Jan 7, 2015
Merged

Assert output types #514

merged 3 commits into from
Jan 7, 2015

Conversation

mappum
Copy link
Contributor

@mappum mappum commented Jan 7, 2015

This fixes a bug in object stat that causes it to error when run locally (not on the daemon). This is because commands must return pointer values (sadly there's no way around it with our crazy interface{} magic in the commands package).

I added a check for this that will panic if a developer makes their command return a non-pointer to prevent the mistake from happening in the future.

@btc btc added the status/in-progress In progress label Jan 7, 2015
@jbenet
Copy link
Member

jbenet commented Jan 7, 2015

so much type-system circumvention. i've little context from commands to usefully CR this -- should i just merge it @mappum?

@mappum
Copy link
Contributor Author

mappum commented Jan 7, 2015

It's definitely not go idiomatic, but it works. Yeah, it's solid and mergeable.

jbenet added a commit that referenced this pull request Jan 7, 2015
@jbenet jbenet merged commit 5853a25 into master Jan 7, 2015
@jbenet jbenet removed the status/in-progress In progress label Jan 7, 2015
@jbenet jbenet deleted the assert-types branch January 7, 2015 23:52
@jbenet jbenet restored the assert-types branch January 8, 2015 02:59
@jbenet
Copy link
Member

jbenet commented Jan 8, 2015

This was reverted (per #516 (comment)) -- let's fix + try again

@ghost ghost deleted the assert-types branch January 22, 2017 04:04
@aschmahmann aschmahmann mentioned this pull request Aug 23, 2021
62 tasks
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.

None yet

3 participants