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

Guide: functions #15314

Closed
wants to merge 1 commit into from
Closed

Conversation

steveklabnik
Copy link
Member

Just a few words about functions and defining them.

```

This is the simplest possible function declaration. As we mentioned before,
`fn` says 'this is a function,' followed by the name, some parenthesis becuase

Choose a reason for hiding this comment

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

s/becuase/because/

I SHOULD COMMENT ON HTE DIFF INSTEAD OF TWEETING. ❤️

Copy link

Choose a reason for hiding this comment

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

s/becuase/because.

Also it''s not clear that "some parenthesis because this function does not return anything" makes sense. The parenthesis are for arguments not return values, no?

@steveklabnik
Copy link
Member Author

Also, freshly rebased on master. Such a weird conflict...

@steveklabnik
Copy link
Member Author

Okay, this has been squashed and is ready to go. r?

```

Using a `return` as the last line of a function works, but is considered poor
style:
Copy link
Member

Choose a reason for hiding this comment

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

I've been wondering about the style guidelines here; e.g. if exceptions should be made for cases where after the return blah one has some fn item declarations that are used by the code above; or even just the cases where the function body is long and so one uses return just to emphasize that you've got to the end.

Anyway, I'll agree that the use of return is poor style in the provided example, so I guess this comment can stay as it is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we will see if any exceptions develop.

@pnkfelix
Copy link
Member

pnkfelix commented Jul 3, 2014

@steveklabnik well, once again, just one nit for you to fix. (Its an easy one, just remove some text that huon and you discussed above.)

The change is otherwise in LGTM state; I'll r-plus after the requested revision (and associated squash).

Just a few words about functions and defining them.
@steveklabnik
Copy link
Member Author

Fixed and squashed.

bors added a commit that referenced this pull request Jul 3, 2014
Closes #15276 (Guide: if)
Closes #15280 (std::os - Add join_paths, make setenv non-utf8 capable)
Closes #15314 (Guide: functions)
Closes #15327 (Simplify PatIdent to contain an Ident rather than a Path)
Closes #15340 (Guide: add mutable binding section)
Closes #15342 (Fix ICE with nested macro_rules!-style macros)
Closes #15350 (Remove duplicated slash in install script path)
Closes #15351 (correct a few spelling mistakes in the tutorial)
Closes #15352 (librustc: Have the kind checker check sub-bounds in trait casts.)
Closes #15359 (Fix spelling errors.)
Closes #15361 (Rename set_broadast() to set_broadcast().)
Closes #15366 (Simplify creating a parser from a token tree)
Closes #15367 (Add examples for StrVector methods)
Closes #15372 (Vec::grow should use reserve_additional, Vec::reserve should check against capacity)
Closes #15373 (Fix minor issues in the documentation of libtime.)
@bors bors closed this in #15377 Jul 3, 2014
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.

7 participants