-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
feat: adds a name field to the server object #4469
Conversation
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Could you please add a (Adding it to the first one should avoid merge conflicts with #4456) |
Co-authored-by: Ralf Handl <ralf.handl@sap.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
…Specification into feat/server-name
Thanks for the quick review! I have made the requested updates. |
@baywet this makes |
@handrews I was wondering that myself as I put the pull request together. So I went and looked for precedent work, like the parameter name, the license name, schema properties name, etc... As for code generation, the tool can always run some kind of sanitation routine based on their own domain restrictions. This is why I decided to leave it open, but happy to narrow it down based on this audience's feedback. |
@baywet thanks for the research and summary. If the TSC is fine with continuing in the same vein as existing name fields, I have no objection. We have had some complaints in this area (which I'm too lazy to dig up right now), but choosing the right restrictions is not an obvious thing. On the other hand, we should be a lot more clear in Moonwalk, particularly with full Unicode support being something people now expect. |
I have no objections to either using the field name |
@lornajane correct, name is just informative as far as I understood the initial issue, no reference mechanism from the spec. It may be used in code generation or to produce other generated artifacts (docs, etc...) at which point we should probably leave that up to the tool consuming it. |
Do we want to keep server objects as an array, rather than moving to an object now that we have a key to index each object by? Right now there is an implied order in the objects (where the first one is attempted to be matched first), but we're not explicit about that in the spec, nor do we say anything about whether we should handle multiple entries matching. If only one may match, then order is irrelevant and we can move to an object. |
Yes, please! Adding an optional fixed field in a minor version is a non-breaking change that will not invalidate any existing OAD if its Changing the structure of Assume you just want to use the improved tags, and now you have to change other completely unrelated things just to pass validation again - very annoying. |
@ralfhandl yes I more meant that we might want to consider changing this in moonwalk :) |
fixes #1821
Tick one of the following options: