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 TypeSpec Definition for APIView TreeStyle Parser #8657

Conversation

chidozieononiwu
Copy link
Member

  • TypeSpec Definition for APIView TreeStyle Parser

Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

I'm confused. I was expecting this PR to contain two things: 1) the TypeSpec definition for APIView and 2) the generated JSON schema of the same.

This seems to be an outline of a TypeSpec library for APIView, but I don't understand why, or what it is doing?

@chidozieononiwu chidozieononiwu force-pushed the 071624.06/TreeStyleParserDocumentation branch 2 times, most recently from a3413fb to 2e97a17 Compare July 18, 2024 17:27
@chidozieononiwu chidozieononiwu self-assigned this Jul 18, 2024
@chidozieononiwu chidozieononiwu force-pushed the 071624.06/TreeStyleParserDocumentation branch 2 times, most recently from 3abc447 to c9a26c6 Compare July 22, 2024 19:35
Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

Overall this is a great step forward. Two things I would like to see:

  1. a script of some kind (though preferably not Powershell) that would allow you to evaluate a provided tokenfile against the generated JSON schema. Every inner-loop dev will need to do that, so that should live here.
  2. Some kind of automated test that would compare test tokenfiles against both the JSON schema and the server parsing logic. This will help ensure that if the server logic changes, that the TypeSpec and JSON schema and likewise updated.

@tjprescott
Copy link
Member

Overall this is a great step forward. Two things I would like to see:

  1. a script of some kind (though preferably not Powershell) that would allow you to evaluate a provided tokenfile against the generated JSON schema. Every inner-loop dev will need to do that, so that should live here.

@chidozieononiwu the JSON registry that Jonathan was mentioning will probably suffice for this, which is great. It will also simplify ask 2.

@chidozieononiwu chidozieononiwu force-pushed the 071624.06/TreeStyleParserDocumentation branch from c9a26c6 to f5d1de7 Compare July 24, 2024 22:34
Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

Approving to work in a more incremental way, but @tjprescott point about incorporating the TypeSpec and schema into automation should be taken into account seriously
CC @maririos @weshaggard

@chidozieononiwu
Copy link
Member Author

Approving to work in a more incremental way, but @tjprescott point about incorporating the TypeSpec and schema into automation should be taken into account seriously CC @maririos @weshaggard

I have updated the tracking issue
#8517

@chidozieononiwu chidozieononiwu merged commit 2e2e0a6 into Azure:main Aug 1, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants