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

OpenAPI: add several features #3258

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

segoon
Copy link

@segoon segoon commented Jan 6, 2022

No description provided.

@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f3da80e) 85.30% compared to head (9470b79) 85.30%.
Report is 1354 commits behind head on master.

Files Patch % Lines
parsers/openapi.c 88.88% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3258   +/-   ##
=======================================
  Coverage   85.30%   85.30%           
=======================================
  Files         208      208           
  Lines       49507    49510    +3     
=======================================
+ Hits        42230    42233    +3     
  Misses       7277     7277           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@masatake
Copy link
Member

masatake commented Jan 7, 2022

Thank you.
You used yaml: as the prefix of the commit header.
Could you use the name of parser instead?

OpenAPI: add several features

@segoon segoon changed the title yaml: add several OpenAPI features OpenAPI: add several features Jan 7, 2022
@segoon
Copy link
Author

segoon commented Jan 7, 2022

@masatake , renamed
also removed 'info' parsing as it is not very informative

@masatake
Copy link
Member

masatake commented Jan 9, 2022

I'm reading https://spec.openapis.org/oas/latest.html slowly.
I use this comment area for taking note about "What should be tagged" in an IDEAL parser.

  • 4.8.2 Info Object
    • title should be tagged.
  • 4.8.5 Server Object
    • url should be tagged.
    • variable should be tagged. A tag for a variable must have the url in its scope.
  • 4.8.6 Server Variable Object
    • enum elements should be tagged. A tag for them must have the variable in its scope.
  • 4.8.8.1 Patterned Fields
    • path should be tagged.
  • 4.8.10 Operation Object
    • operationId must be tagged. It may have operation: parser specific field; its value is one of "get", "put", ...
  • 4.8.11 External Documentation Object
    • url can be extracted as a reference tag.
  • 4.8.12 Parameter Object
    • name should be tagged. It may have in: parser specific field. scope: field should be filled. schema may be stored to typeref: field. style is interesting. However, I'm not sure how to store it in a field of a tag entry for name.
  • 4.8.13 Request Body Object
    This is an interesting item. However, I cannot find a good way to handle this in ctags.

(to be continued...)

masatake pushed a commit to masatake/ctags that referenced this pull request Jan 9, 2022
This change is derrived from universal-ctags#3258.
The original commit is so large.
@masatake splited the commit smaller per-topic ones.
@masatake wrote this commit log.
masatake pushed a commit to masatake/ctags that referenced this pull request Jan 9, 2022
The original code tried matching even after making a tag for a token.

This change is derrived from universal-ctags#3258.
The original commit is so large.
@masatake splited the commit smaller per-topic ones.
@masatake wrote this commit log.
masatake pushed a commit to masatake/ctags that referenced this pull request Jan 9, 2022
This change is derrived from universal-ctags#3258.
The original commit is so large.
@masatake splited the commit smaller per-topic ones.
@masatake wrote this commit log.
masatake pushed a commit to masatake/ctags that referenced this pull request Jan 9, 2022
This change is derrived from universal-ctags#3258.
The original commit is so large.
@masatake splited the commit smaller per-topic ones.
@masatake wrote this commit log.
masatake pushed a commit to masatake/ctags that referenced this pull request Jan 9, 2022
This change is derrived from universal-ctags#3258.
The original commit is so large.
@masatake splited the commit smaller per-topic ones.
@masatake wrote this commit log.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake masatake mentioned this pull request Jan 9, 2022
masatake added a commit that referenced this pull request Jan 10, 2022
@segoon
Copy link
Author

segoon commented Jan 22, 2022

@masatake hi! What should we do with this PR? Do you expect me to fix the conflict, or you want to e.g. split it into parts and merge the parts by yourself or...?

@masatake
Copy link
Member

Sorry for making you wait. But could you wait for a while? I would like to finish the 'Basic' parser related task first.

@segoon
Copy link
Author

segoon commented Jan 25, 2022

Yeah, no problem! Thx

@masatake
Copy link
Member

Do you expect me to fix the conflict, or do you want to e.g. split it into parts and merge the parts by yourself or...?

I would like to do merge some of the parts (and kinds).
I would like you to know how I do; I will add kinds incrementally.

I will make a pull request for merging. I would like you to review the pull request.
After that, could you try doing the same as I do for the rest of the parts?

@segoon
Copy link
Author

segoon commented Jan 30, 2022

@masatake OK, seems fair. I'll try to split it following the necessary patch "granulation".

masatake pushed a commit to masatake/ctags that referenced this pull request Feb 7, 2022
@segoon
Copy link
Author

segoon commented Mar 26, 2022

@masatake friendly ping. Should I update this PR and show you for code review, or do you want to do it by yourself?

@@ -53,3 +55,65 @@ components:
schema:
type: object
properties: {}
examples:
Copy link
Member

@masatake masatake Mar 26, 2022

Choose a reason for hiding this comment

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

X I'm talking about these.

@masatake
Copy link
Member

masatake commented Mar 26, 2022

I'm sorry to be late. I'm thinking about how to make the Yaml parser more extensible.
Can I ask you to update your PR? In that case, I would like you to do:

(1) split the commit into smaller commits; each commit deals one kind, and
(2) prioritize the commits.

If you want to have a kind X in your parser strongly, please put the commit for the kind in front of the commits of the pull request.

I stopped my review when thinking about the "example" kind (See X).
I cannot convince myself whether we should make tags for the examples.

If the pull request is separated into commits per kind, I can merge ones obviously useful.

@segoon
Copy link
Author

segoon commented Mar 26, 2022

OK, I'll try it the next week.

@masatake masatake mentioned this pull request Nov 3, 2023
@masatake masatake added this to the 6.2 milestone Dec 28, 2023
@masatake masatake modified the milestones: 6.2, 6.3 Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants