search

Ogeon / rustful

[OUTDATED] A light HTTP framework for Rust
https://docs.rs/rustful
Apache License 2.0
862 stars 52 forks source link

Rename Context.uri to Context.path #118

Closed hjr3 closed 8 years ago

hjr3 commented 8 years ago

BREAKING CHANGE

Context.uri was misleading as it represented the path of the URI and not the entire URI. The Uri type was renamed to UriPath and Context.uri was renamed to Context.path.

This change modifies the public interface of rustful. Anyone depending on Context.uri will encounter compiler errors.

Fixes #115

hjr3 commented 8 years ago

@Ogeon my first PR into this project to get my feet wet. I don't love the name UriPath, but I could not come up with a better one. Calling it just Path made the code read weird. In fact, there are already places where you see context.path.as_path() which is a little weird too.

Ogeon commented 8 years ago

Yeah, naming is hard, and I can't come up with anything better at the moment. It looks great, otherwise, and the PR description is top class. It's just that detail in the doc comments.

Ogeon commented 8 years ago

Thinking about it, maybe uri_path isn't too bad... What do you think?

hjr3 commented 8 years ago

@Ogeon Thank you for the feedback. The documentation is fixed. I think uri_path is a better name. I made all the changes. If that looks good, I will squash the commits together.

Ogeon commented 8 years ago

Looks great!

hjr3 commented 8 years ago

@Ogeon squashed

Ogeon commented 8 years ago

Alright, thank you!

@homu r+

homu commented 8 years ago

:pushpin: Commit d62c282 has been approved by Ogeon

homu commented 8 years ago

:zap: Test exempted - status