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.

Sign up for GitHub

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

Jump to bottom

Rename EndpointResult to Result #240

Closed
wants to merge 5 commits into from
Closed

Rename EndpointResult to Result #240

wants to merge 5 commits into from

Conversation

k-nasa
Copy link
Contributor

@k-nasa k-nasa commented May 20, 2019

Description

Rename EndptResult to Result.

Motivation and Context

#235

How Has This Been Tested?

Compile all and run test code

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

prasannavl
prasannavl reviewed May 20, 2019
View reviewed changes
Copy link
Contributor

@prasannavl prasannavl left a comment

Choose a reason for hiding this comment

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

This is mesmerising, and so nice! However, I just filed this: http-rs/http-service#32

I guess I'm going to be against the popular opinion here, as I'm afraid of the same.

@k-nasa
Copy link
Contributor Author

k-nasa commented May 21, 2019

@prasannavl In the example, I think it is more intuitive to return a simple Result. However, for tide developers, it may not be desirable to change "EndpointResult" to "Result".

I have this idea. By defining tide :: Result as an alias of EndpointResult, user use 'Result', tide developer use EndpointResult. What do you think?

@k-nasa
Copy link
Contributor Author

k-nasa commented May 25, 2019

@prasannavl ?

@prasannavl
Copy link
Contributor

Hi @k-nasa - Sorry, been a bit busy. I'm keen to see that others think of this. Meanwhile #235 (comment).

@k-nasa
Copy link
Contributor Author

k-nasa commented May 26, 2019

@prasannavl Ok. thanks! And sorry to rush

@k-nasa k-nasa closed this Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prasannavl prasannavl prasannavl left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k-nasa @prasannavl