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

Add a FAQ #550

Merged
merged 4 commits into from
Apr 27, 2018
Merged

Add a FAQ #550

merged 4 commits into from
Apr 27, 2018

Conversation

achew22
Copy link
Collaborator

@achew22 achew22 commented Feb 16, 2018

@lukasmalkmus, can you please review this? I would love your input.

Fixes #525

Copy link
Contributor

@lukasmalkmus lukasmalkmus left a comment

Choose a reason for hiding this comment

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

I would just shorten the questions a little bit. This FAQ should serve well enough for now. We could however open an issue for discussion on this matter and link it here? This would allow other users to easily find and contribute to the discussion.

README.md Outdated
@@ -239,6 +239,16 @@ But patch is welcome.
# Contribution
See [CONTRIBUTING.md](http://github.com/grpc-ecosystem/grpc-gateway/blob/master/CONTRIBUTING.md).

# FAQ

> Why are the models in the swagger spec prefixed with the last bit of the proto package name? Is there a reason for this? I find this pretty ugly tbh and don’t see the advantage.
Copy link
Contributor

@lukasmalkmus lukasmalkmus Feb 16, 2018

Choose a reason for hiding this comment

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

I would shorten the question to: Why are the models in the swagger specification prefixed with the last part of the proto package name?

README.md Outdated

The reason to generate the prefixes is that we don't have a guaranteed unique namespace. If two packages produce different Foo messages then we will have trouble.

> Why not strip the prefix if it isn't necessary?
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest shortening: Why not strip the prefix?

README.md Outdated

> Why not strip the prefix if it isn't necessary?

I tried that, but the problem is that if you do that then when you add a message in that happens to conflict it will break code that is very far away from the code that changed. This is in an effort to adhere to the [principle of least astonishment](https://en.wikipedia.org/wiki/Principle_of_least_astonishment).
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a bit more general:

When a message is added which happens to conflict with another message (e.g. by importing a message with the same name from a different package) it will break code that is very far away from the code that changed. This is in an effort to adhere to the [principle of least astonishment](https://en.wikipedia.org/wiki/Principle_of_least_astonishment).

Moving it into its own file in the docs section
@achew22
Copy link
Collaborator Author

achew22 commented Apr 27, 2018

@lukasmalkmus, sorry for being so slow to respond. What do you think about my updated FAQ entries?

@lukasmalkmus
Copy link
Contributor

No worries. Looks good.

@achew22 achew22 merged commit 09ecb99 into master Apr 27, 2018
@achew22 achew22 deleted the faq branch April 27, 2018 23:17
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants