Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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(spec): tag POC #435
feat(spec): tag POC #435
Changes from 2 commits
7671b0a
9dbc271
432b8bb
d16f2db
8ad4f6f
521b914
dd4f6e0
b8a8342
1cd50b7
4ae5f9e
d2cefcc
fd0ace9
6899303
de0c17a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I there a reason we want to ensure that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because if you don't define tags at the root level then nothing will appear in the doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will still default to the base tag so I guess it's fine, what I mean is this feature is for the doc and shouldn't prevent dev but it's just my opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once it's done it's done, if you add a tag that means you expect to change the doc. So I'm not too worried about this impacting dev flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to avoid any manual changes added in the specs, and default to the name of the folder the spec is stored in?
This way, we only have to handle where to put the file, not what tag it should match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So with today's structure, the tag for
getTask
would beadvanced
, but we can also create a new folder forindices
, and we default to itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope because of
deleteIndex
andsaveObject
living in the same rest pathThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sorry I did not fully explained why, but looking at the rest path,
search
settings
objects
all have the same so I guess we can do whatever we want here with the folders, wdyt?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could but it would need more care in
spec.yml
, experience tells me deducting something from the filesystem will blow up on your face at some point.And
deleteIndex
andsaveObject
are not splittable because of the limitation of$ref
object, so my point still stand.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are both index operations so it wouldn't choc me to see both of them under an
index
folder/tag.My point is only that we don't have extra logic but I get yours too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one is index operation, the other record operation. it's definitely debatable but as a user I would find it weird :p