Skip to content

Example code improvement #3

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

Closed
wants to merge 1 commit into from
Closed

Example code improvement #3

wants to merge 1 commit into from

Conversation

GuillaumeGomez
Copy link
Member

cc @gkoz

@gkoz
Copy link
Member

gkoz commented Sep 6, 2015

I'm not enthusiastic about endorsing .ok().expect(...)...

@GuillaumeGomez
Copy link
Member Author

Hum... Any particular reason ?

@gkoz
Copy link
Member

gkoz commented Sep 6, 2015

It works here because our error type is () but generally is inferior to Result::expect and seems the opposite of idiomatic. People will use pieces of our examples without questioning them. Maybe we should replace the panic with print! and return.

@GuillaumeGomez
Copy link
Member Author

I'd prefer this actually. I'm not a big fan of panic! when it's not absolutely necessary.

@gkoz
Copy link
Member

gkoz commented Sep 6, 2015

We should start by updating the actual example(s) anyway and then bring the change here.

@GuillaumeGomez
Copy link
Member Author

Actually, it doesn't seem to be a priority but if you have nothing else waiting, then I let you update examples. ;)

@gkoz
Copy link
Member

gkoz commented Sep 6, 2015

I'm working on push some hand-generated docs after that maybe.

@GuillaumeGomez
Copy link
Member Author

As you wish, I'm starting to prepare to push on crates.io tomorrow.

@gkoz
Copy link
Member

gkoz commented Sep 6, 2015

I've found a little issue with the version handling and enums in cairo. Might want to hold off pusblishing crates a bit.

@GuillaumeGomez
Copy link
Member Author

Ok, I'm in standby. Just email me when the situation is unlocked please. ^^

@gkoz
Copy link
Member

gkoz commented Sep 6, 2015

I've just learnt more about the issue. Good news is, it's not a crates showstopper.

Turns out rustdoc doesn't get the same cfg directives rustc gets so it can't see any of the gated stuff. WAT

@GuillaumeGomez
Copy link
Member Author

DARK MAGIC !!!
Well, I work on crates tomorrow now. Witches will wait !

@gkoz
Copy link
Member

gkoz commented Nov 28, 2015

Looks like we've resolved this in a different way.

@gkoz gkoz closed this Nov 28, 2015
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