-
Notifications
You must be signed in to change notification settings - Fork 22
Feature/add properties v3 #171
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
Feature/add properties v3 #171
Conversation
-> Add new constructor in Field -> Delete class FieldSchema
Let me know when you want a review. I'll need a detailed description of your aims though. Oh and you need to pass CI too (see below).
Release note
- Add Size for string (maxLength)
- Add Note in Diagram (in Note there are Description, Example, Format, Extension)
- Add field in Class in addition link (like Diagram Class)
- Replace {O} (optional) by {R} (Require)
- There is a case on error when "array", so the type return object[]
Example
image
Looks interesting. Have you checked the appearance of the other diagrams in demos? What's good for you might be a whole lot of mess for others. Probably boils down to configuration options which you have a TODO for. Default will probably be to leave that stuff out.
- Extend properties on other case - Add example
An option should be added to display the notes (yellow tag) (see TODO)
If we compare with before, links are lost
I'll study
I think I'm done.
Would you be interested in my contribution?
I remain at your disposal for any questions.
Hi Dimitri
Thanks for getting involved. My first comment is that there are too many enhancements mixed up in this PR. For the sanity of the reviewers you need to keep changes separate so that they can be discussed and worked on independently of other changes. This is something you pick up as you get into open-source contributions.
This is your summary:
- Add Size for string (maxLength)
- Add Note in Diagram (in Note there are Description, Example, Format, Extension)
- Add field in Class in addition link (like Diagram Class)
- Replace {O} (optional) by {R} (Require)
- There is a case on error when "array", so the type return object[]
Each of those contributions should be a separate PR but the best idea is to discuss what you want to do first so you don't put too much effort in to something that doesn't get accepted.
The output of this project is a UML Diagram that communicates the essence of the API, not the details. I'm not happy with the addition of string maxLength and notes because it seems like needless clutter. For example with the notes, in general the times that a description adds more than just the existing name of the field is when the description is long and detailed. In that specific case I also don't want that appearing as extra clutter. Here's a quick example of a long description (my apis have lots of long descriptions) that I definitely would not want in a diagram!
name: continuationToken
description: |
Describes to the server the starting point of
the next page of results and is obtained from
the current page. May contain an offset if desired
but is at the discretion of implementer. Note that
it is possible that a call specifying a continuation
token may return en empty list (but an empty list return
should not have a continuation token on it so at
that point paging would stop).
The {O} vs {R} usage is a style thing. Where have you seen {R} usage? Got links? I find {O} usage less noisy than {R} usage in general and that is apparent in the bookstore diagram above.
Add field in Class in addition link (like Diagram Class)
I need more info on this one.
There is a case on error when "array", so the type return object[]
I need more info on this effects of this one
So I'm not open to adding Note, size, and {R} instead of {O}. I don't see them as adding value but you can try and convince me!
The other two issues need more explanation and should be opened as separate PRs please.
I see your PR does not pass CI. This mean that running mvn clean install
on the project is failing. Make sure you do that check before requesting review.
Another tip for PR contributions is to do all your work in your fork of the repo and only open a PR on the original repo when your PR is ready for review. That way I won't be drawn by notifications to inspect your changes only to see they are not ready for review.
No description provided.