Skip to content

Conversation

@abhishekkumams
Copy link
Contributor

@abhishekkumams abhishekkumams commented Oct 20, 2022

Why make this change?

  • It contains updating the design doc to understand the changes that are required for implementing stored-procedure for graphql
  • It also helps understand the rationale for certain assumptions that were made.

What is this change?

NOTE:

the doc contains images that might not be shown correctly on the Files changed tab. To view the images correctly head over to the branch file: https://github.com/Azure/data-api-builder/blob/dev/abhishekkuma/graphql-support-for-stored-procedure-doc/docs/internals/StoredProcedureDesignDoc.md

@Aniruddh25 Aniruddh25 linked an issue Oct 28, 2022 that may be closed by this pull request
@abhishekkumams abhishekkumams marked this pull request as draft October 31, 2022 05:42
@abhishekkumams abhishekkumams marked this pull request as ready for review November 25, 2022 09:23
Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Some changes requested to be clearer in explanations.
One general comment: why is much of this markdown document formatted as quotes using the ">" character?

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for keeping our documentation up to date!

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

There's a thread about JsonDocument -> JsonElement that wasn't changed. Can you also note why many sections of this markdown document are quoted with the > character?

@abhishekkumams
Copy link
Contributor Author

There's a thread about JsonDocument -> JsonElement that wasn't changed. Can you also note why many sections of this markdown document are quoted with the > character?

Updated to JsonElement.

most part of the document is quoted as '>' because this document was initially designed in that way to show every subsection to appear differently. I also think it makes it easier to follow each section.

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Only reason I bring > up is because it is markdown block quote syntax and those sections are not quotes. I understand that they may be more readable, but I wonder if there is a better way to make it more readable.
Quotes make me think "this content is being quoted from somewhere... where is that?"

@abhishekkumams
Copy link
Contributor Author

Only reason I bring > up is because it is markdown block quote syntax and those sections are not quotes. I understand that they may be more readable, but I wonder if there is a better way to make it more readable. Quotes make me think "this content is being quoted from somewhere... where is that?"

okay, I see your point now.

@abhishekkumams abhishekkumams merged commit 18d29fb into main Dec 6, 2022
@abhishekkumams abhishekkumams deleted the dev/abhishekkuma/graphql-support-for-stored-procedure-doc branch December 6, 2022 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation graphql

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Stored Procedure Support

5 participants